[
https://issues.apache.org/jira/browse/CALCITE-3877?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17695424#comment-17695424
]
Gian Merlino commented on CALCITE-3877:
---------------------------------------
I wanted to inquire about this block added in SqlToRelConverter as part of this
patch:
{code}
if (!aggCall.getOperator().allowsFraming()) {
// If the operator does not allow framing, bracketing is implicitly
// everything up to the current row.
sqlLowerBound = SqlWindow.createUnboundedPreceding(SqlParserPos.ZERO);
sqlUpperBound = SqlWindow.createCurrentRow(SqlParserPos.ZERO);
if (aggCall.getKind() == SqlKind.ROW_NUMBER) {
// ROW_NUMBER() expects specific kind of framing.
rows = true;
}
}
{code}
The comment and logic doesn't seem right. The operators that do not allow
framing are {{LAG}}, {{LEAD}}, {{RANK}}, {{ROW_NUMBER}}, {{PERCENT_RANK}},
{{DENSE_RANK}}, and {{CUME_DIST}}. However certain of these operators, like
{{LAG}} and {{LEAD}}, are meant to "see" the entire window, including rows
beyond the current row.
Would it make sense for me to raise a patch changing the upper bound in all
these cases to unbounded following?
Btw, I noticed this when upgrading to the latest version of Calcite, and
noticing that the default frames changed for {{LAG}} and {{LEAD}}. It was
formerly unbounded preceding to unbounded following.
> In RexWindow, make fields upperBound and lowerBound not-nullable
> ----------------------------------------------------------------
>
> Key: CALCITE-3877
> URL: https://issues.apache.org/jira/browse/CALCITE-3877
> Project: Calcite
> Issue Type: Bug
> Reporter: Steven Talbot
> Assignee: Julian Hyde
> Priority: Major
> Fix For: 1.23.0
>
>
> Null lowerBound/upperBond in RexWindow causes NPE in RexShuttle. Here's a
> test that can be dropped into RexShuttleTest.java:
> {code:java}
> @Test public void handlesNullBoundsInRexWindow() {
> final RelBuilder builder =
> RelBuilder.create(RelBuilderTest.config().build());
> final RexNode over = builder.getRexBuilder().makeOver(
> builder.getTypeFactory().createSqlType(SqlTypeName.BIGINT),
> SqlStdOperatorTable.ROW_NUMBER,
> ImmutableList.of(),
> ImmutableList.of(),
> ImmutableList.of(),
> null,
> null,
> true,
> true,
> true,
> true,
> true
> );
> assertThat(over.accept(new RexShuttle()), is(over));
> }
> {code}
> That will raise an NPE. Fix is to handle NULLs when doing the child accept on
> the RexWindowBound[s] in RexShuttle.visitWindow.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
