Skip to content

[CALCITE-2421] Improve RexSimplify when unknownAsFalse is true#765

Open
laurentgo wants to merge 1 commit intoapache:mainfrom
laurentgo:laurentgo/CALCITE-2421
Open

[CALCITE-2421] Improve RexSimplify when unknownAsFalse is true#765
laurentgo wants to merge 1 commit intoapache:mainfrom
laurentgo:laurentgo/CALCITE-2421

Conversation

@laurentgo
Copy link
Copy Markdown
Contributor

Improve simplification of expression by RexSimplify when unknownAsFalse
is true by not setting back unknownAsFalse to false while processing
terms.

Allow for expressions like A = A AND B = B to be simplified as
A IS NOT NULL AND B IS NOT NULL

When simplifying AND expression with unknownAsFalse set to true,
simplification of some terms might be under-optimal as unknownAsFalse
value is not propagated through but set to false.

Expressions like A <= A AND B => B might stay as-is whereas they could
be simplified as A IS NOT NULL AND B IS NOT NULL

Improve the simplification of those comparison expressions when
unknownAsFalse is true by keeping flag as-is while processing terms in
simplifyAndTerms.

private void simplifyAndTerms(List<RexNode> terms) {
RexSimplify simplify = withUnknownAsFalse(false);
RexSimplify simplify = this;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This change was already implemented in 6b3844c, however the test fails.
@laurentgo , could you please rebase?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, looks like already fixed. Should I still work on adding the test cases?

@vlsi vlsi added the returned-with-feedback There are review comments (in JIRA and/or in GitHub) to be implemented before merging the PR label Jan 8, 2019
@danny0405 danny0405 force-pushed the master branch 2 times, most recently from 80f411d to ca27fe9 Compare November 30, 2019 07:52
@vlsi vlsi force-pushed the master branch 2 times, most recently from 49cb002 to 8768a23 Compare December 29, 2019 12:07
@julianhyde julianhyde force-pushed the main branch 2 times, most recently from 8a5cf83 to cf7f71b Compare June 8, 2023 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

returned-with-feedback There are review comments (in JIRA and/or in GitHub) to be implemented before merging the PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants