[fix](nereids) not rewrite first_value when first_value ignore nulls #45065
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
feiniaofeiafei
force-pushed
the
fix_window_fe
branch
from
6239af7 to
8c163a1
Compare
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
TPC-H: Total hot run time: 39776 ms |
TPC-DS: Total hot run time: 190850 ms |
ClickBench: Total hot run time: 31.96 s |
morrySnow
reviewed
Dec 5, 2024
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/WindowFunctionChecker.java
Outdated
Show resolved
Hide resolved
16 tasks
|
run cloud_p0 |
1 similar comment
|
run cloud_p0 |
feiniaofeiafei
force-pushed
the
fix_window_fe
branch
2 times, most recently
from
9e92b94 to
1b7d53f
Compare
|
run buildall |
feiniaofeiafei
force-pushed
the
fix_window_fe
branch
2 times, most recently
from
33fe9d3 to
457d16d
Compare
|
run buildall |
TPC-H: Total hot run time: 39933 ms |
TPC-DS: Total hot run time: 197176 ms |
ClickBench: Total hot run time: 31.95 s |
feiniaofeiafei
force-pushed
the
fix_window_fe
branch
from
457d16d to
286d2c8
Compare
|
run buildall |
morrySnow
reviewed
Dec 9, 2024
Comment on lines
320
to
322
| if (!firstValue.child(1).isConstant()) { | ||
| throw new AnalysisException("The second parameter of first_value must be constant"); | ||
| } |
There was a problem hiding this comment.
maybe we should add this check in org.apache.doris.nereids.trees.expressions.functions.window.FirstOrLastValue.
let impl checkLegalityBeforeTypeCoercion
TPC-H: Total hot run time: 39821 ms |
TPC-DS: Total hot run time: 197688 ms |
ClickBench: Total hot run time: 32.63 s |
feiniaofeiafei
force-pushed
the
fix_window_fe
branch
from
286d2c8 to
ec75860
Compare
feiniaofeiafei
force-pushed
the
fix_window_fe
branch
from
4b6689a to
83969fd
Compare
|
run buildall |
TPC-H: Total hot run time: 40115 ms |
TPC-DS: Total hot run time: 197242 ms |
ClickBench: Total hot run time: 33.02 s |
seawinde
approved these changes
Dec 10, 2024
github-actions
bot
removed
the
approved
morrySnow
approved these changes
Dec 10, 2024
|
PR approved by at least one committer and no changes requested. |
github-actions
bot
added
the
approved
yiguolei
pushed a commit
that referenced
this pull request
Dec 11, 2024
…gnore nulls #45065 (#45236) Cherry-picked from #45065 Co-authored-by: feiniaofeiafei <[email protected]>
BiteTheDDDDt
pushed a commit
that referenced
this pull request
Dec 12, 2024
…44996) ### What problem does this PR solve? Problem Summary: needs another prs of FE: #45065 #45264 1. the first value even if have set value, should not return directly, need check it whether need arg_ignore_null, as maybe it's NULL 2. the last_value if need arg_ignore_null and not set value, should find in while loop, if find could return directly, not find check iff has any value before ### Release note fix first_value/last_value return error with ignore null param is true
github-actions bot
pushed a commit
that referenced
this pull request
Dec 12, 2024
…44996) ### What problem does this PR solve? Problem Summary: needs another prs of FE: #45065 #45264 1. the first value even if have set value, should not return directly, need check it whether need arg_ignore_null, as maybe it's NULL 2. the last_value if need arg_ignore_null and not set value, should find in while loop, if find could return directly, not find check iff has any value before ### Release note fix first_value/last_value return error with ignore null param is true
github-actions bot
pushed a commit
that referenced
this pull request
Dec 12, 2024
…44996) ### What problem does this PR solve? Problem Summary: needs another prs of FE: #45065 #45264 1. the first value even if have set value, should not return directly, need check it whether need arg_ignore_null, as maybe it's NULL 2. the last_value if need arg_ignore_null and not set value, should find in while loop, if find could return directly, not find check iff has any value before ### Release note fix first_value/last_value return error with ignore null param is true
zhangstar333
pushed a commit
that referenced
this pull request
Dec 16, 2024
…gnore nulls #45065 (#45235) Cherry-picked from #45065 Co-authored-by: feiniaofeiafei <[email protected]>
yiguolei
pushed a commit
that referenced
this pull request
Dec 16, 2024
…44996) ### What problem does this PR solve? Problem Summary: needs another prs of FE: #45065 #45264 1. the first value even if have set value, should not return directly, need check it whether need arg_ignore_null, as maybe it's NULL 2. the last_value if need arg_ignore_null and not set value, should find in while loop, if find could return directly, not find check iff has any value before ### Release note fix first_value/last_value return error with ignore null param is true
zhangstar333
added a commit
to zhangstar333/incubator-doris
that referenced
this pull request
Dec 16, 2024
…pache#44996) ### What problem does this PR solve? Problem Summary: needs another prs of FE: apache#45065 apache#45264 1. the first value even if have set value, should not return directly, need check it whether need arg_ignore_null, as maybe it's NULL 2. the last_value if need arg_ignore_null and not set value, should find in while loop, if find could return directly, not find check iff has any value before ### Release note fix first_value/last_value return error with ignore null param is true
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
this pr fix 2 things:
1.when first_value/last_value second parameter is true, which means ignore nulls, fe should not do some rewrite.
2.adding check that first_value/last_value second parameter must be true or false.
Issue Number: close #xxx
Related PR: #44996 #27623
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)