Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify conditional catalyst expressions generated by udf-compiler #5315

Merged

Conversation

seanprime7
Copy link
Contributor

if (c) true else false => c
if (c) false else true => !c

Signed-off-by: Sean Lee selee@nvidia.com

Resolves #3985

@jlowe jlowe added this to the Apr 18 - Apr 29 milestone Apr 26, 2022
@seanprime7 seanprime7 force-pushed the seanprime7/udf-compiler-conditional-simplification branch from 633771a to 4afaaf8 Compare April 26, 2022 16:43
jlowe
jlowe previously approved these changes Apr 26, 2022
@jlowe
Copy link
Member

jlowe commented Apr 26, 2022

build

abellina
abellina previously approved these changes Apr 26, 2022
Copy link
Collaborator

@abellina abellina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM other than @gerashegalov's suggestions.

if (c) true else false => c
if (c) false else true => !c

Signed-off-by: Sean Lee <selee@nvidia.com>
@seanprime7 seanprime7 dismissed stale reviews from abellina and jlowe via bba7ec3 April 26, 2022 21:16
@seanprime7 seanprime7 force-pushed the seanprime7/udf-compiler-conditional-simplification branch from 4afaaf8 to bba7ec3 Compare April 26, 2022 21:16
Copy link
Collaborator

@gerashegalov gerashegalov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jlowe
Copy link
Member

jlowe commented Apr 26, 2022

@seanprime7 please do not force-push once the PR is being reviewed. It often orphans the reviewer comments and makes re-reviews more difficult. Commits are already squashed when merged, so PRs don't need to optimize their commits before merging.

@seanprime7
Copy link
Contributor Author

copy that

@jlowe
Copy link
Member

jlowe commented Apr 26, 2022

build

@sameerz sameerz added the feature request New feature or request label Apr 27, 2022
@jlowe jlowe merged commit 8877b33 into branch-22.06 Apr 27, 2022
@seanprime7 seanprime7 deleted the seanprime7/udf-compiler-conditional-simplification branch May 3, 2022 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEA] UDF-Compiler: Translation of simple predicate UDF should allow predicate pushdown
5 participants