[Feature] Support Coalesce for Column Names #9327

61yao · 2022-09-02T03:10:52Z

Coalesce takes a list of arguments and returns the first not null value. If all arguments are null, return a null.

This implementations transform all arguments into string and return the first non-null string value. Null is represented as string value "null"

codecov-commenter · 2022-09-02T03:50:29Z

Codecov Report

Merging #9327 (43ac34c) into master (ff2a333) will increase coverage by 3.56%.
The diff coverage is 87.50%.

@@             Coverage Diff              @@
##             master    #9327      +/-   ##
============================================
+ Coverage     63.40%   66.97%   +3.56%     
- Complexity     4762     4894     +132     
============================================
  Files          1832     1403     -429     
  Lines         98146    73108   -25038     
  Branches      15020    11722    -3298     
============================================
- Hits          62231    48964   -13267     
+ Misses        31321    20582   -10739     
+ Partials       4594     3562    -1032

Flag	Coverage Δ
unittests1	`66.97% <87.50%> (+0.05%)`	⬆️
unittests2	`?`

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

Impacted Files	Coverage Δ
.../transform/function/CoalesceTransformFunction.java	`87.35% <87.35%> (ø)`
...e/pinot/common/function/TransformFunctionType.java	`100.00% <100.00%> (ø)`
...r/transform/function/TransformFunctionFactory.java	`86.89% <100.00%> (+0.09%)`	⬆️
.../org/apache/pinot/query/routing/WorkerManager.java	`83.87% <0.00%> (-6.87%)`	⬇️
...he/pinot/segment/local/segment/store/IndexKey.java	`65.00% <0.00%> (-5.00%)`	⬇️
.../aggregation/function/ModeAggregationFunction.java	`88.10% <0.00%> (ø)`
...ller/recommender/data/generator/TimeGenerator.java
...ontroller/api/resources/PinotControllerLogger.java
...inion/event/DefaultMinionEventObserverFactory.java
...ker/routing/instanceselector/InstanceSelector.java
... and 430 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

...c/main/java/org/apache/pinot/core/operator/transform/function/CoalesceTransformFunction.java

siddharthteotia · 2022-09-07T09:26:45Z

...c/main/java/org/apache/pinot/core/operator/transform/function/CoalesceTransformFunction.java

I think implicitly treating everything as STRING will give confusing semantics imo. If we don't want to support this function on few data types, then throwing exception is better.

Reference - https://docs.snowflake.com/en/sql-reference/functions/coalesce.html

+1. For the first version, we can check if all the input are numbers or strings

I added a check for argument type. but the return value has to be a string, otherwise we are not able to represent null?

Good point. Currently we don't support null values for transform function yet. You may read TransformBlockValSet.getNullBitmap(), which count the result as null when any argument is null. This won't work for coalesce.

For now, we may use NullValueUtils.getDefaultNullValue() for each data type to present the null. Then we should figure out a way to support the real null in transform function.

(edited) represented using default nullvalue probably is not a good idea since all numeric data type uses zero as default null.

@walterddr Not really. We use min value as the default for numeric types except for big decimal, which doesn't have a min value

Updated to use NullValueUtils.getDefaultNullValue() and only supports numeric and string types. Also it requires the type to be same for all arguments.

...c/main/java/org/apache/pinot/core/operator/transform/function/CoalesceTransformFunction.java

Jackie-Jiang · 2022-09-13T07:27:52Z

...c/main/java/org/apache/pinot/core/operator/transform/function/CoalesceTransformFunction.java

I don't think we need this check. We may use getResultMetadata() to rule out the unsupported types

Removed the second check but we need the type to be identifier to get the null bit map?

...c/main/java/org/apache/pinot/core/operator/transform/function/CoalesceTransformFunction.java

Jackie-Jiang

LGTM with minor comments

Jackie-Jiang · 2022-09-14T03:30:27Z

...c/main/java/org/apache/pinot/core/operator/transform/function/CoalesceTransformFunction.java

(minor) Let's change the name to storedType to be more explicit

Jackie-Jiang · 2022-09-14T03:32:40Z

...c/main/java/org/apache/pinot/core/operator/transform/function/CoalesceTransformFunction.java

(minor) We usually follow the sequence of INT, LONG, FLOAT, DOUBLE, BIG_DECIMAL, STRING (same order as the enum for easier tracking

(also use intellij autocomplete to generate the branches will automatically be in that order :-) )

Jackie-Jiang · 2022-09-14T03:33:57Z

...c/main/java/org/apache/pinot/core/operator/transform/function/CoalesceTransformFunction.java

(minor) Let's calculate the result metadata in the init() and store it in a member variable. It has 2 benefits:

Fail fast when the type cannot be supported

Prevent calculating result metadata multiple times

Jackie-Jiang · 2022-09-14T03:35:21Z

...c/main/java/org/apache/pinot/core/operator/transform/function/CoalesceTransformFunction.java

(minor) Follow the same sequence (INT, LONG, FLOAT, DOUBLE, BIG_DECIMAL, STRING) as the interface for easier tracking

Jackie-Jiang · 2022-09-14T03:35:45Z

...c/main/java/org/apache/pinot/core/operator/transform/function/CoalesceTransformFunction.java

(minor) Put this method before getDoublelTransformResults() for easier tracking

siddharthteotia reviewed Sep 7, 2022

View reviewed changes

...c/main/java/org/apache/pinot/core/operator/transform/function/CoalesceTransformFunction.java Outdated Show resolved Hide resolved

siddharthteotia reviewed Sep 7, 2022

View reviewed changes

61yao force-pushed the coalesce branch 3 times, most recently from adbc410 to 21d21ae Compare September 10, 2022 01:19

Jackie-Jiang reviewed Sep 13, 2022

View reviewed changes

Jackie-Jiang approved these changes Sep 14, 2022

View reviewed changes

Yao Liu added 5 commits September 14, 2022 14:53

test

959d32c

coalesce

814c237

use bitmap from projection and check argument types

407c29f

Handle different types of Coalesce

0aa8d30

add double float support

015d94f

61yao force-pushed the coalesce branch from fb3c559 to 7c80923 Compare September 14, 2022 21:53

address comments

43ac34c

61yao force-pushed the coalesce branch from 7c80923 to 43ac34c Compare September 14, 2022 21:57

Merge branch 'master' into coalesce

69589be

walterddr merged commit a949f95 into apache:master Sep 14, 2022

Jackie-Jiang mentioned this pull request Jul 14, 2025

Support: IS DISTINCT FROM operator, IS NOT DISTINCT FROM operator, and COALESCE transform #9218

Closed

[Feature] Support Coalesce for Column Names #9327

[Feature] Support Coalesce for Column Names #9327

Uh oh!

Conversation

61yao commented Sep 2, 2022

Uh oh!

codecov-commenter commented Sep 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

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!

walterddr Sep 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Jackie-Jiang left a comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov-commenter commented Sep 2, 2022 •

edited

Loading

walterddr Sep 9, 2022 •

edited

Loading