Skip to content

Conversation

@yashmayya
Copy link
Contributor

  • Similar to Support arbitrary number of WHEN THEN clauses in the scalar CASE function #14125, but for COALESCE.
  • For the single-stage engine, this fix is only relevant for post-aggregation (since other parts of the query can always use the transform function implementation of COALESCE that already supports an arbitrary number of arguments) - so something like COALESCE(SUM(col1), SUM(col2), SUM(col3), SUM(col4), SUM(col5), SUM(col6) wouldn't work before this fix.
  • For the multi-stage engine, however, this fix is more important since any function call in the intermediate stages can only use the scalar function equivalent.

@yashmayya yashmayya added bugfix multi-stage Related to the multi-stage query engine enhancement labels Oct 9, 2024
@codecov-commenter
Copy link

codecov-commenter commented Oct 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.80%. Comparing base (59551e4) to head (fb96c61).
Report is 1170 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #14195      +/-   ##
============================================
+ Coverage     61.75%   63.80%   +2.05%     
- Complexity      207     1534    +1327     
============================================
  Files          2436     2622     +186     
  Lines        133233   144396   +11163     
  Branches      20636    22103    +1467     
============================================
+ Hits          82274    92133    +9859     
- Misses        44911    45464     +553     
- Partials       6048     6799     +751     
Flag Coverage Δ
custom-integration1 100.00% <ø> (+99.99%) ⬆️
integration 100.00% <ø> (+99.99%) ⬆️
integration1 100.00% <ø> (+99.99%) ⬆️
integration2 0.00% <ø> (ø)
java-11 63.79% <ø> (+2.08%) ⬆️
java-21 63.69% <ø> (+2.06%) ⬆️
skip-bytebuffers-false 63.80% <ø> (+2.05%) ⬆️
skip-bytebuffers-true 63.66% <ø> (+35.93%) ⬆️
temurin 63.80% <ø> (+2.05%) ⬆️
unittests 63.80% <ø> (+2.05%) ⬆️
unittests1 55.46% <ø> (+8.57%) ⬆️
unittests2 34.36% <ø> (+6.62%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yashmayya yashmayya force-pushed the coalesce-scalar-variadic branch from 5a635ed to 03aaf50 Compare October 9, 2024 16:10
Copy link
Contributor

@Jackie-Jiang Jackie-Jiang left a comment

Choose a reason for hiding this comment

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

LGTM!

@Nullable
private static Object coalesceVar(Object... objects) {
@ScalarFunction(nullableParameters = true, isVarArg = true)
public static Object coalesce(Object... objects) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's keep the @Nullable annotation since it could return null

@Jackie-Jiang Jackie-Jiang merged commit 40ea3f9 into apache:master Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix enhancement multi-stage Related to the multi-stage query engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants