-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[fix](nereids) Fix not in aggregate's output err after eliminate by uniform when group sets exist #56942
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
[fix](nereids) Fix not in aggregate's output err after eliminate by uniform when group sets exist #56942
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
TPC-DS: Total hot run time: 190484 ms |
ClickBench: Total hot run time: 30.57 s |
FE Regression Coverage ReportIncrement line coverage |
80639d5 to
28c4a89
Compare
|
run buildall |
TPC-DS: Total hot run time: 189982 ms |
ClickBench: Total hot run time: 30.27 s |
FE Regression Coverage ReportIncrement line coverage |
| LogicalRepeat<?> sourceRepeat = (LogicalRepeat<?>) exprIdReplacer.rewriteExpr( | ||
| aggregate.getSourceRepeat().get(), replaceMap); | ||
| aggregate = aggregate.withSourceRepeat(sourceRepeat); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe this will lead to agg source repeat be rewritten twice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not same instance, so doesn't repeat be rewritten twice.
| } | ||
| List<Expression> newChildren = new ArrayList<>(); | ||
| List<Expression> oldChildren = originExpression.get().children(); | ||
| for (Expression child : oldChildren) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- directly traverse the originExpression instead of using for loop traversing originExpression children maybe better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have fixed
| newChildren.add(child.accept(new DefaultExpressionRewriter<Void>() { | ||
| @Override | ||
| public Expression visitSlotReference(SlotReference slot, Void context) { | ||
| ExprId newId = replaceMap.get(slot.getExprId()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these code is repeated, use a function to avoid repeated code may be better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have fixed
| }).toRule(ExpressionRuleType.EXPR_ID_REWRITE_REPLACE) | ||
| }).toRule(ExpressionRuleType.EXPR_ID_REWRITE_REPLACE), | ||
| matchesType(VirtualSlotReference.class).thenApply(ctx -> { | ||
| VirtualSlotReference virtualSlot = ctx.expr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1.there is an attribute "realExpressions" in VirtualSlotReference, should we replace "realExpressions" too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
realExpressions is derived from original Expressions, we have replaced original Expressions rightly, realExpressions is right auto
…niform when group sets exist
28c4a89 to
7e9d158
Compare
|
run buildall |
TPC-DS: Total hot run time: 190540 ms |
ClickBench: Total hot run time: 27.49 s |
FE Regression Coverage ReportIncrement line coverage |
|
run buildall |
TPC-DS: Total hot run time: 190091 ms |
ClickBench: Total hot run time: 27.57 s |
FE UT Coverage ReportIncrement line coverage |
|
PR approved by anyone and no changes requested. |
|
PR approved by at least one committer and no changes requested. |
…niform when group sets exist (#56942) Fix not in aggregate's output err after eliminate by uniform when group sets exist if query as following, would cause `ERROR 1105 (HY000): errCode = 2, detailMessage = GROUPING_PREFIX_event_name_group not in aggregate's output` the pr fix this ```sql SELECT CASE WHEN GROUPING(event_date) = 1 THEN '(TOTAL)' ELSE CAST(event_date AS VARCHAR) END AS event_date, user_id, MAX(conversion_level) AS conversion_level, CASE WHEN GROUPING(event_name_group) = 1 THEN '(TOTAL)' ELSE event_name_group END AS event_name_group FROM ( SELECT src.event_date, src.user_id, WINDOW_FUNNEL( 3600 * 24 * 1, 'default', src.event_time, src.event_name = 'shop_buy', src.event_name = 'shop_buy' ) AS conversion_level, src.event_name_group FROM ( SELECT CAST(etb.`@dt` AS DATE) AS event_date, etb.`@event_name` AS event_name, etb.`@event_time` AS event_time, etb.`@event_name` AS event_name_group, etb.`@user_id` AS user_id FROM `test_event` AS etb WHERE etb.`@dt` between '2025-09-03 02:00:00' AND '2025-09-10 01:59:59' AND etb.`@event_name` = 'shop_buy' AND etb.`@user_id` IS NOT NULL AND etb.`@user_id` > '0' ) AS src GROUP BY src.event_date, src.user_id, src.event_name_group ) AS fwt GROUP BY GROUPING SETS ( (user_id), (user_id, event_date), (user_id, event_name_group), (user_id, event_date, event_name_group) ); ``` ### What problem does this PR solve? Issue Number: close #xxx Related PR: #43391 Problem Summary: ### Release note None ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [ ] 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 <!-- Add your reason? --> - Behavior changed: - [ ] No. - [ ] Yes. <!-- Explain the behavior change --> - Does this need documentation? - [ ] No. - [ ] Yes. <!-- Add document PR link here. eg: apache/doris-website#1214 --> ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into -->
…niform when group sets exist (#56942) Fix not in aggregate's output err after eliminate by uniform when group sets exist if query as following, would cause `ERROR 1105 (HY000): errCode = 2, detailMessage = GROUPING_PREFIX_event_name_group not in aggregate's output` the pr fix this ```sql SELECT CASE WHEN GROUPING(event_date) = 1 THEN '(TOTAL)' ELSE CAST(event_date AS VARCHAR) END AS event_date, user_id, MAX(conversion_level) AS conversion_level, CASE WHEN GROUPING(event_name_group) = 1 THEN '(TOTAL)' ELSE event_name_group END AS event_name_group FROM ( SELECT src.event_date, src.user_id, WINDOW_FUNNEL( 3600 * 24 * 1, 'default', src.event_time, src.event_name = 'shop_buy', src.event_name = 'shop_buy' ) AS conversion_level, src.event_name_group FROM ( SELECT CAST(etb.`@dt` AS DATE) AS event_date, etb.`@event_name` AS event_name, etb.`@event_time` AS event_time, etb.`@event_name` AS event_name_group, etb.`@user_id` AS user_id FROM `test_event` AS etb WHERE etb.`@dt` between '2025-09-03 02:00:00' AND '2025-09-10 01:59:59' AND etb.`@event_name` = 'shop_buy' AND etb.`@user_id` IS NOT NULL AND etb.`@user_id` > '0' ) AS src GROUP BY src.event_date, src.user_id, src.event_name_group ) AS fwt GROUP BY GROUPING SETS ( (user_id), (user_id, event_date), (user_id, event_name_group), (user_id, event_date, event_name_group) ); ``` ### What problem does this PR solve? Issue Number: close #xxx Related PR: #43391 Problem Summary: ### Release note None ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [ ] 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 <!-- Add your reason? --> - Behavior changed: - [ ] No. - [ ] Yes. <!-- Explain the behavior change --> - Does this need documentation? - [ ] No. - [ ] Yes. <!-- Add document PR link here. eg: apache/doris-website#1214 --> ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into -->
…niform when group sets exist (apache#56942) Fix not in aggregate's output err after eliminate by uniform when group sets exist if query as following, would cause `ERROR 1105 (HY000): errCode = 2, detailMessage = GROUPING_PREFIX_event_name_group not in aggregate's output` the pr fix this ```sql SELECT CASE WHEN GROUPING(event_date) = 1 THEN '(TOTAL)' ELSE CAST(event_date AS VARCHAR) END AS event_date, user_id, MAX(conversion_level) AS conversion_level, CASE WHEN GROUPING(event_name_group) = 1 THEN '(TOTAL)' ELSE event_name_group END AS event_name_group FROM ( SELECT src.event_date, src.user_id, WINDOW_FUNNEL( 3600 * 24 * 1, 'default', src.event_time, src.event_name = 'shop_buy', src.event_name = 'shop_buy' ) AS conversion_level, src.event_name_group FROM ( SELECT CAST(etb.`@dt` AS DATE) AS event_date, etb.`@event_name` AS event_name, etb.`@event_time` AS event_time, etb.`@event_name` AS event_name_group, etb.`@user_id` AS user_id FROM `test_event` AS etb WHERE etb.`@dt` between '2025-09-03 02:00:00' AND '2025-09-10 01:59:59' AND etb.`@event_name` = 'shop_buy' AND etb.`@user_id` IS NOT NULL AND etb.`@user_id` > '0' ) AS src GROUP BY src.event_date, src.user_id, src.event_name_group ) AS fwt GROUP BY GROUPING SETS ( (user_id), (user_id, event_date), (user_id, event_name_group), (user_id, event_date, event_name_group) ); ``` ### What problem does this PR solve? Issue Number: close #xxx Related PR: apache#43391 Problem Summary: ### Release note None ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [ ] 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 <!-- Add your reason? --> - Behavior changed: - [ ] No. - [ ] Yes. <!-- Explain the behavior change --> - Does this need documentation? - [ ] No. - [ ] Yes. <!-- Add document PR link here. eg: apache/doris-website#1214 --> ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into -->
…niform when group sets exist (apache#56942) Fix not in aggregate's output err after eliminate by uniform when group sets exist if query as following, would cause `ERROR 1105 (HY000): errCode = 2, detailMessage = GROUPING_PREFIX_event_name_group not in aggregate's output` the pr fix this ```sql SELECT CASE WHEN GROUPING(event_date) = 1 THEN '(TOTAL)' ELSE CAST(event_date AS VARCHAR) END AS event_date, user_id, MAX(conversion_level) AS conversion_level, CASE WHEN GROUPING(event_name_group) = 1 THEN '(TOTAL)' ELSE event_name_group END AS event_name_group FROM ( SELECT src.event_date, src.user_id, WINDOW_FUNNEL( 3600 * 24 * 1, 'default', src.event_time, src.event_name = 'shop_buy', src.event_name = 'shop_buy' ) AS conversion_level, src.event_name_group FROM ( SELECT CAST(etb.`@dt` AS DATE) AS event_date, etb.`@event_name` AS event_name, etb.`@event_time` AS event_time, etb.`@event_name` AS event_name_group, etb.`@user_id` AS user_id FROM `test_event` AS etb WHERE etb.`@dt` between '2025-09-03 02:00:00' AND '2025-09-10 01:59:59' AND etb.`@event_name` = 'shop_buy' AND etb.`@user_id` IS NOT NULL AND etb.`@user_id` > '0' ) AS src GROUP BY src.event_date, src.user_id, src.event_name_group ) AS fwt GROUP BY GROUPING SETS ( (user_id), (user_id, event_date), (user_id, event_name_group), (user_id, event_date, event_name_group) ); ``` Issue Number: close #xxx Related PR: apache#43391 Problem Summary: None - Test <!-- At least one of them must be included. --> - [ ] 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 <!-- Add your reason? --> - Behavior changed: - [ ] No. - [ ] Yes. <!-- Explain the behavior change --> - Does this need documentation? - [ ] No. - [ ] Yes. <!-- Add document PR link here. eg: apache/doris-website#1214 --> - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into -->
…niform when group sets exist (apache#56942) Fix not in aggregate's output err after eliminate by uniform when group sets exist if query as following, would cause `ERROR 1105 (HY000): errCode = 2, detailMessage = GROUPING_PREFIX_event_name_group not in aggregate's output` the pr fix this ```sql SELECT CASE WHEN GROUPING(event_date) = 1 THEN '(TOTAL)' ELSE CAST(event_date AS VARCHAR) END AS event_date, user_id, MAX(conversion_level) AS conversion_level, CASE WHEN GROUPING(event_name_group) = 1 THEN '(TOTAL)' ELSE event_name_group END AS event_name_group FROM ( SELECT src.event_date, src.user_id, WINDOW_FUNNEL( 3600 * 24 * 1, 'default', src.event_time, src.event_name = 'shop_buy', src.event_name = 'shop_buy' ) AS conversion_level, src.event_name_group FROM ( SELECT CAST(etb.`@dt` AS DATE) AS event_date, etb.`@event_name` AS event_name, etb.`@event_time` AS event_time, etb.`@event_name` AS event_name_group, etb.`@user_id` AS user_id FROM `test_event` AS etb WHERE etb.`@dt` between '2025-09-03 02:00:00' AND '2025-09-10 01:59:59' AND etb.`@event_name` = 'shop_buy' AND etb.`@user_id` IS NOT NULL AND etb.`@user_id` > '0' ) AS src GROUP BY src.event_date, src.user_id, src.event_name_group ) AS fwt GROUP BY GROUPING SETS ( (user_id), (user_id, event_date), (user_id, event_name_group), (user_id, event_date, event_name_group) ); ``` ### What problem does this PR solve? Issue Number: close #xxx Related PR: apache#43391 Problem Summary: ### Release note None ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [ ] 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 <!-- Add your reason? --> - Behavior changed: - [ ] No. - [ ] Yes. <!-- Explain the behavior change --> - Does this need documentation? - [ ] No. - [ ] Yes. <!-- Add document PR link here. eg: apache/doris-website#1214 --> ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into -->
…niform when group sets exist apache#56942 (apache#5783) pr: apache#56942 commitId: a5e4e1c
…niform when group sets exist (apache#56942) (apache#5631) pr: apache#56942 commitId: apache@961cef6 ## Proposed changes Issue Number: close #xxx <!--Describe your changes.-->
Fix not in aggregate's output err after eliminate by uniform when group sets exist
if query as following, would cause
ERROR 1105 (HY000): errCode = 2, detailMessage = GROUPING_PREFIX_event_name_group not in aggregate's outputthe pr fix this
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #43391
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)