-
Notifications
You must be signed in to change notification settings - Fork 2.5k
[CALCITE-7701] Support IGNORE NULLS for FIRST_VALUE/LAST_VALUE window functions in the enumerable convention #5178
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,6 +35,7 @@ | |
| import org.apache.calcite.linq4j.tree.OptimizeShuttle; | ||
| import org.apache.calcite.linq4j.tree.ParameterExpression; | ||
| import org.apache.calcite.linq4j.tree.Primitive; | ||
| import org.apache.calcite.linq4j.tree.Types; | ||
| import org.apache.calcite.linq4j.tree.UnsignedType; | ||
| import org.apache.calcite.rel.type.RelDataType; | ||
| import org.apache.calcite.rel.type.RelDataTypeFactory; | ||
|
|
@@ -2489,12 +2490,107 @@ | |
| AggResultContext result) { | ||
| WinAggResultContext winResult = (WinAggResultContext) result; | ||
|
|
||
| final boolean ignoreNulls = | ||
| info instanceof WinAggContext && ((WinAggContext) info).ignoreNulls(); | ||
| if (ignoreNulls) { | ||
| return implementResultIgnoreNulls(info, winResult); | ||
| } | ||
|
|
||
| return Expressions.condition(winResult.hasRows(), | ||
| winResult.rowTranslator( | ||
| winResult.computeIndex(Expressions.constant(0), seekType)) | ||
| .translate(winResult.rexArguments().get(0), info.returnType()), | ||
| getDefaultValue(info.returnType())); | ||
| } | ||
|
|
||
| /** | ||
| * Implements FIRST_VALUE / LAST_VALUE with IGNORE NULLS by scanning the | ||
| * frame (forward for FIRST_VALUE, backward for LAST_VALUE) and returning | ||
| * the first non-null argument value, or null if all rows in the frame are | ||
| * null (or the frame is empty). | ||
| * | ||
| * <p>Generated code (for FIRST_VALUE; LAST_VALUE scans backward): | ||
| * <pre>{@code | ||
| * BoxType res = null; | ||
| * if (hasRows) { | ||
| * for (int seekIdx = startIndex; seekIdx <= endIndex; seekIdx++) { | ||
| * BoxType seekValue = rowTranslator.translate(arg, boxType); | ||
| * if (seekValue != null) { | ||
| * res = seekValue; | ||
| * break; | ||
| * } | ||
| * } | ||
| * } | ||
| * return res; | ||
| * }</pre> | ||
| */ | ||
| private Expression implementResultIgnoreNulls(AggContext info, | ||
| WinAggResultContext winResult) { | ||
| final Type returnType = info.returnType(); | ||
| final RexNode arg = winResult.rexArguments().get(0); | ||
|
|
||
| // Use a boxed type internally so that a NULL comparison is always valid, | ||
| // even when the (frame-guaranteed non-empty) return type is a primitive. | ||
| // The surrounding window implementation converts the result back to the | ||
| // declared return type. | ||
| final Type boxType = Types.box(returnType); | ||
|
|
||
| final ParameterExpression res = | ||
| Expressions.parameter(0, boxType, | ||
| winResult.currentBlock().newName( | ||
| seekType == SeekType.START ? "first_value" : "last_value")); | ||
| // res = null | ||
| winResult.currentBlock().add(Expressions.declare(0, res, NULL_EXPR)); | ||
|
|
||
| final ParameterExpression idx = | ||
| Expressions.parameter(int.class, | ||
| winResult.currentBlock().newName("seekIdx")); | ||
|
|
||
| // Scan direction: FIRST_VALUE walks from start to end, LAST_VALUE walks | ||
| // from end back to start. | ||
| final boolean forward = seekType == SeekType.START; | ||
| final Expression from = | ||
| forward ? winResult.startIndex() : winResult.endIndex(); | ||
| final Expression to = | ||
| forward ? winResult.endIndex() : winResult.startIndex(); | ||
| final Expression condition = | ||
| forward | ||
| ? Expressions.lessThanOrEqual(idx, to) | ||
| : Expressions.greaterThanOrEqual(idx, to); | ||
| final Expression post = | ||
| forward | ||
| ? Expressions.postIncrementAssign(idx) | ||
| : Expressions.postDecrementAssign(idx); | ||
|
|
||
| // Build the loop body: | ||
| // BoxType seekValue = rowTranslator.translate(arg, boxType); | ||
|
Check warning on line 2566 in core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
|
||
| // if (seekValue != null) { | ||
| // res = seekValue; | ||
| // break; | ||
| // } | ||
| final BlockBuilder loopBody = winResult.nestBlock(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add comments showing the generated Java code for each of these statements?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, I had added comments showing the generated code. |
||
| final Expression value = | ||
| winResult.rowTranslator(idx).translate(arg, boxType); | ||
| final ParameterExpression valueVar = | ||
| Expressions.parameter(0, boxType, loopBody.newName("seekValue")); | ||
| loopBody.add(Expressions.declare(0, valueVar, value)); | ||
| loopBody.add( | ||
| Expressions.ifThen( | ||
| Expressions.notEqual(valueVar, NULL_EXPR), | ||
| Expressions.block( | ||
| Expressions.statement(Expressions.assign(res, valueVar)), | ||
| Expressions.break_(null)))); | ||
| winResult.exitBlock(); | ||
| final BlockStatement loopBodyBlock = loopBody.toBlock(); | ||
|
|
||
| // Wrap the scan in: if (hasRows) { for (...) { ... } } | ||
|
Check warning on line 2586 in core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
|
||
| winResult.currentBlock().add( | ||
| Expressions.ifThen(winResult.hasRows(), | ||
| Expressions.for_( | ||
| Expressions.declare(0, idx, from), | ||
| condition, post, loopBodyBlock))); | ||
| return res; | ||
| } | ||
| } | ||
|
|
||
| /** Implementor for the {@code FIRST_VALUE} windowed aggregate function. */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1323,4 +1323,76 @@ java.sql.SQLException: Error while executing SQL "select first_value(sal) filter | |
| from emp": FILTER clause is not supported for window function FIRST_VALUE | ||
| !error | ||
|
|
||
| # [CALCITE-7701] Support IGNORE NULLS for FIRST_VALUE/LAST_VALUE window functions in the enumerable convention | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These SQL programs had been validated in Oracle: https://onecompiler.com/oracle/44xqcbrnw |
||
| # Verified against Oracle | ||
| # FIRST_VALUE with IGNORE NULLS returns the first non-null value in the frame | ||
| # (or NULL if the frame is empty or all values are null). | ||
| select o, v, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. all these tests have a finite window with ROWS. |
||
| first_value(v) ignore nulls over (order by o rows 2 preceding) as fv | ||
| from (values (1, 1), (2, cast(null as integer)), (3, 3), | ||
| (4, cast(null as integer)), (5, cast(null as integer))) as t(o, v); | ||
| +---+---+----+ | ||
| | O | V | FV | | ||
| +---+---+----+ | ||
| | 1 | 1 | 1 | | ||
| | 2 | | 1 | | ||
| | 3 | 3 | 1 | | ||
| | 4 | | 3 | | ||
| | 5 | | 3 | | ||
| +---+---+----+ | ||
| (5 rows) | ||
|
|
||
| !ok | ||
|
|
||
| # LAST_VALUE with IGNORE NULLS returns the last non-null value in the frame. | ||
| select o, v, | ||
| last_value(v) ignore nulls over (order by o rows 2 preceding) as lv | ||
| from (values (1, 1), (2, cast(null as integer)), (3, 3), | ||
| (4, cast(null as integer)), (5, cast(null as integer))) as t(o, v); | ||
| +---+---+----+ | ||
| | O | V | LV | | ||
| +---+---+----+ | ||
| | 1 | 1 | 1 | | ||
| | 2 | | 1 | | ||
| | 3 | 3 | 3 | | ||
| | 4 | | 3 | | ||
| | 5 | | 3 | | ||
| +---+---+----+ | ||
| (5 rows) | ||
|
|
||
| !ok | ||
|
|
||
| # IGNORE NULLS returns NULL when every row in the frame is null. | ||
| select o, v, | ||
| first_value(v) ignore nulls | ||
| over (order by o rows between 1 preceding and 1 preceding) as fv | ||
| from (values (1, cast(null as integer)), (2, cast(null as integer)), | ||
| (3, 5)) as t(o, v); | ||
| +---+---+----+ | ||
| | O | V | FV | | ||
| +---+---+----+ | ||
| | 1 | | | | ||
| | 2 | | | | ||
| | 3 | 5 | | | ||
| +---+---+----+ | ||
| (3 rows) | ||
|
|
||
| !ok | ||
|
|
||
| # RESPECT NULLS (the default) still returns the boundary value, including NULL. | ||
| select o, v, | ||
| first_value(v) respect nulls over (order by o rows 2 preceding) as fv, | ||
| last_value(v) over (order by o rows 2 preceding) as lv | ||
| from (values (1, 1), (2, cast(null as integer)), (3, 3)) as t(o, v); | ||
| +---+---+----+----+ | ||
| | O | V | FV | LV | | ||
| +---+---+----+----+ | ||
| | 1 | 1 | 1 | 1 | | ||
| | 2 | | 1 | | | ||
| | 3 | 3 | 1 | 3 | | ||
| +---+---+----+----+ | ||
| (3 rows) | ||
|
|
||
| !ok | ||
|
|
||
| # End winagg.iq | ||
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.
It seems to be that this is the default value, so it is NULL.
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.
Yes, the NULL_EXPR passed to Expressions.declare initializes res to null, which is the default value returned when the frame is empty or every row in the frame is null. This matches the behavior in the non-IGNORE NULLS path, where getDefaultValue(info.returnType()) also produces null for these functions. I kept the code as-is because the surrounding Javadoc now explicitly says "or null if all rows in the frame are null", so the intent should be clear from the documentation.