Skip to content

branch-2.1: [fix](nereids) do eliminate constant group by key in normalizeagg #49589 #50212

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.

Sign up for GitHub

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

Jump to bottom
Merged
yiguolei merged 4 commits into from
May 8, 2025
Merged

branch-2.1: [fix](nereids) do eliminate constant group by key in normalizeagg #49589 #50212

yiguolei merged 4 commits into from
May 8, 2025

Conversation

@feiniaofeiafei
Copy link
Contributor

@feiniaofeiafei feiniaofeiafei commented Apr 21, 2025

Cherry-picked from #49589

@hello-stephen
Copy link
Contributor

hello-stephen commented Apr 21, 2025

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:

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

@feiniaofeiafei
Copy link
Contributor Author

feiniaofeiafei commented Apr 21, 2025

run buildall

@feiniaofeiafei feiniaofeiafei changed the title branch-3.0: [fix](nereids) do eliminate constant group by key in normalizeagg #49589 branch-2.1: [fix](nereids) do eliminate constant group by key in normalizeagg #49589 Apr 21, 2025
@feiniaofeiafei
Copy link
Contributor Author

feiniaofeiafei commented Apr 21, 2025

run buildall

1 similar comment
@feiniaofeiafei
Copy link
Contributor Author

feiniaofeiafei commented May 7, 2025

run buildall

@feiniaofeiafei
[fix](nereids) do eliminate constant group by key in normalizeagg (ap…
8601a3b 
…ache#49589)

Related PR: apache#32878 apache#49473

Problem Summary:

 SELECT
    IF(
        t.`gender` IN ('女'),
        (
            TIMESTAMPDIFF(
                YEAR,
                NOW(),
                NOW()
            )
        ),
        1
    ) AS x0,
    TIMESTAMPDIFF(
        YEAR,
        NOW(),
        NOW()
    ) AS x1
FROM
t1 AS t
GROUP BY
    x0,
    x1;

after EliminateGroupByConstant, this sql will be rewritten to

 SELECT
    IF(
        t.`gender` IN ('女'),
        0,
        1
    ) AS x0,
    0 AS x1
FROM
t1 AS t
GROUP BY
     IF(
        t.`gender` IN ('女'),
        (
            TIMESTAMPDIFF(
                YEAR,
                NOW(),
                NOW()
            )
        ),
        1
    ) ;

The select expression and the group by expression is different, and will
report error in normalizeagg.

The fix in PR apache#49473 may introduce another issue. Consider the following
query:

SELECT func2(100) FROM t GROUP BY func1(), func2(func1());

If func1() can be constant-folded to 100, then func2(func1()) will be
replaced with func2(100), allowing the query to execute successfully.
However, when func1() cannot be folded to 100, the query will fail. This
creates an inconsistent behavior where query execution depends on
whether func1() can be constant-folded or not, which is not an ideal
implementation.

To address this issue, this PR modifies the normalizeAgg logic to
eliminate constant group by keys. With this change, the query will
consistently fail regardless of whether func1() can be folded or not,
ensuring more predictable behavior.
@feiniaofeiafei
fix
0be7d25
@feiniaofeiafei
fix regression
526e209
@feiniaofeiafei
fix feut
8b610b3
@feiniaofeiafei
Copy link
Contributor Author

feiniaofeiafei commented May 8, 2025

run buildall

@feiniaofeiafei
Copy link
Contributor Author

feiniaofeiafei commented May 8, 2025

run p0

1 similar comment
@feiniaofeiafei
Copy link
Contributor Author

feiniaofeiafei commented May 8, 2025

run p0

@yiguolei yiguolei merged commit ebe302c into apache:branch-2.1 May 8, 2025
19 checks passed
GoGoWen pushed a commit to GoGoWen/incubator-doris that referenced this pull request Sep 26, 2025
fix(nereids): do eliminate constant group by key in normalizeagg apac…
aea6c9b 
…he#49589 (apache#50212)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yiguolei yiguolei yiguolei approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@feiniaofeiafei @hello-stephen @yiguolei