[feat](nereids) simplify comparison predicate rule add check data type limit by yujun777 · Pull Request #44732 · apache/doris

yujun777 · 2024-11-28T08:35:47Z

What problem does this PR solve?

simplify comparison, check data type's limit, like:

suppose a is tinyint, so its range should be [-128, 127], then we can simplify:

a = -129 => false
cast (a as small int) = small int(-129) => false
a <= -129 => false
a > -129 => true
a <= -128 => a = -128

currently data type check only support tinyint, small int, int, big int, decimalv3.

if data type is float like, compare them with literal may lost precision, but maybe suport it later.

This PR need more test.

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:

Release note

None

Check List (For Author)

Test
- Regression test
- Unit Test
- Manual test (add detailed scripts or steps below)
- No need to test or manual test. Explain why:
  - This is a refactor/code format and no logic has been changed.
  - Previous test can cover this change.
  - No code files have been changed.
  - Other reason
Behavior changed:
- No.
- Yes.
Does this need documentation?
- No.
- Yes.

Check List (For Reviewer who merge this PR)

Confirm the release note
Confirm test cases
Confirm document
Add branch pick label

doris-robot · 2024-11-28T08:35:52Z

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

What problem was fixed (it's best to include specific error reporting information). How it was fixed.
Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
What features were added. Why was this function added?
Which code was refactored and why was this part of the code refactored?
Which functions were optimized and what is the difference before and after the optimization?

yujun777 · 2024-11-28T08:36:57Z

run buildall

yujun777 · 2024-11-28T09:04:49Z

run buildall

morrySnow · 2024-11-28T14:51:46Z

+ }
+ }
+
+ public static Expression getTrue(Expression expression) {


maybe trueOrNull is a better name?

maybe trueOrNull is a better name?
fix

morrySnow · 2024-11-28T14:53:24Z

+ //minVal = BigDecimal.valueOf(-Float.MAX_VALUE);
+ return Optional.of(Pair.of(new BigDecimal(String.valueOf(Float.MIN_VALUE)),
+ new BigDecimal(String.valueOf(Float.MAX_VALUE))));
+ } else if (dataType.isDoubleType()) {
+ //minVal = BigDecimal.valueOf(-Double.MAX_VALUE);


need -Double.MAX_VALUE

need -Double.MAX_VALUE
fix

morrySnow · 2024-11-28T14:57:28Z

+ if (scale >= 0) {
+ StringBuilder sb = new StringBuilder();
+ for (int i = 0; i < precision - scale; i++) {
+ sb.append('9');


maybe u could use org.apache.common.lang3.StringUtils.repeat

maybe u could use org.apache.common.lang3.StringUtils.repeat
fix

yujun777 · 2024-11-29T02:20:31Z

run buildall

yujun777 · 2024-11-29T14:15:45Z

run buildall

yujun777 · 2024-11-29T14:43:37Z

run buildall

yujun777 · 2024-12-02T01:29:13Z

prequire PR: #44831

yujun777 · 2024-12-03T08:15:06Z

run buildall

morrySnow · 2024-12-03T08:27:24Z

--------PhysicalLimit[LOCAL]
----------PhysicalProject
------------PhysicalStorageLayerAggregate[test_pull_up_predicate_literal]
------PhysicalProject


maybe we should modify pull up's case to ensure not generate PhysicalEmptyRelation

morrySnow · 2024-12-03T08:27:45Z

 PhysicalResultSink
--hashJoin[INNER_JOIN] hashCondition=((t1.id = t2.id)) otherCondition=()
----filter((cast(id as BIGINT) = 2147483648))
------PhysicalOlapScan[t1]


same as pull up predicate cases

same as pull up predicate cases
update

yujun777 · 2024-12-03T08:49:40Z

run buildall

github-actions · 2024-12-06T08:22:45Z

PR approved by at least one committer and no changes requested.

github-actions · 2024-12-06T08:22:47Z

PR approved by anyone and no changes requested.

yujun777 · 2024-12-13T07:20:36Z

run buildall

…parison rule (#44920) pick the fix compare bug code from #44732. but #44732 not pick into 3.0

…e limit (apache#44732) simplify comparison, check data type's limit, like: suppose a is tinyint, so its range should be [-128, 127], then we can simplify: a = -129 => false cast (a as small int) = small int(-129) => false a <= -129 => false a > -129 => true a <= -128 => a = -128 currently data type check only support tinyint, small int, int, big int, decimalv3. if data type is float like, compare them with literal may lost precision, but maybe suport it later. This PR need more test.

morrySnow reviewed Nov 28, 2024
View reviewed changes

yujun777 force-pushed the simplify-comparison-with-limit branch from 67fd0d5 to 476a296Compare November 29, 2024 01:40

yujun777 force-pushed the simplify-comparison-with-limit branch 3 times, most recently from c61cf89 to 5517819Compare November 29, 2024 14:05

yujun777 force-pushed the simplify-comparison-with-limit branch from dd010e3 to e5ef895Compare November 29, 2024 14:43

yujun777 force-pushed the simplify-comparison-with-limit branch 2 times, most recently from 4126bab to 7b19651Compare November 30, 2024 05:23

yujun777 marked this pull request as draft December 2, 2024 01:28

yujun777 marked this pull request as ready for review December 3, 2024 08:15

morrySnow reviewed Dec 3, 2024
View reviewed changes

starocean999 previously approved these changes Dec 6, 2024
View reviewed changes

github-actionsBot added the approved Indicates a PR has been approved by one committer. label Dec 6, 2024

github-actionsBot added the reviewed label Dec 6, 2024

englefly previously approved these changes Dec 9, 2024
View reviewed changes

yujun777 mentioned this pull request Dec 9, 2024
branch-3.0: [fix](nereids) fix compare with long min for simplify comparison rule #44920
Merged
16 tasks

yujun777 force-pushed the simplify-comparison-with-limit branch from 828bc82 to a99911eCompare December 13, 2024 07:16

yujun777 dismissed englefly’s stale review via 90382c9December 13, 2024 07:20

yujun777 dismissed starocean999’s stale review via 90382c9December 13, 2024 07:20

yujun777 force-pushed the simplify-comparison-with-limit branch from a99911e to 90382c9Compare December 13, 2024 07:20

add session variable disable_nereids_expression_rules
8201561

yujun777 force-pushed the simplify-comparison-with-limit branch from 90382c9 to 8201561Compare December 13, 2024 07:20

starocean999 approved these changes Dec 13, 2024
View reviewed changes

morrySnow approved these changes Dec 13, 2024
View reviewed changes

morrySnow added the need more test Add more test label Dec 13, 2024

morrySnow merged commit 475571b into apache:masterDec 13, 2024

starocean999 pushed a commit that referenced this pull request Dec 17, 2024
branch-3.0: [fix](nereids) fix compare with long min for simplify com…
0444e6c
…parison rule (#44920) pick the fix compare bug code from #44732. but #44732 not pick into 3.0

Uh oh!

Conversation

yujun777 commented Nov 28, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

Release note

Check List (For Author)

Check List (For Reviewer who merge this PR)

Uh oh!

doris-robot commented Nov 28, 2024

Uh oh!

yujun777 commented Nov 28, 2024

Uh oh!

yujun777 commented Nov 28, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yujun777 commented Nov 29, 2024

Uh oh!

yujun777 commented Nov 29, 2024

Uh oh!

yujun777 commented Nov 29, 2024

Uh oh!

yujun777 commented Dec 2, 2024

Uh oh!

yujun777 commented Dec 3, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yujun777 commented Dec 3, 2024

Uh oh!

github-actionsBot commented Dec 6, 2024

Uh oh!

github-actionsBot commented Dec 6, 2024

Uh oh!

yujun777 commented Dec 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

yujun777 commented Nov 28, 2024•
edited
Loading