[
https://issues.apache.org/jira/browse/CALCITE-6020?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17876118#comment-17876118
]
Mihai Budiu commented on CALCITE-6020:
--------------------------------------
I have already merged your PR in main.
I haven't filed an issue yet with our compiler, since I am still investigating
the exact cause.
The symptom is that building with Calcite using the commit just before your
change everything is fine, but with Calcite including your change the compiled
program fails.
The program that reproduces the issue is the following:
{code:sql}
CREATE TABLE transactions (
cc_num BIGINT NOT NULL,
amt FLOAT64,
unix_time INTEGER NOT NULL
);
CREATE VIEW features as
SELECT
AVG(amt) OVER(
PARTITION BY cc_num
ORDER BY unix_time
RANGE BETWEEN 2592000 PRECEDING AND 1 PRECEDING) AS
avg_spend_pm,
COUNT(*) OVER(
PARTITION BY cc_num
ORDER BY unix_time
RANGE BETWEEN 86400 PRECEDING AND 1 PRECEDING ) AS
trans_freq_24
FROM transactions AS t1;
{code}
What happens using your change is that Calcite asserts that a specific result
is not null, but it turns out to be null at runtime.
The working logical plan is:
LogicalProject(AVG_SPEND_PM=[/(CASE(>($3, 0), $4, null:DOUBLE), $3)],
TRANS_FREQ_24=[$5]), id = 45
LogicalWindow(window#0=[window(partition {0} order by [2] range between
$3 PRECEDING and $4 PRECEDING aggs [COUNT($1), $SUM0($1)])],
window#1=[window(partition {0} order by [2] range between $5 PRECEDING and $4
PRECEDING aggs [COUNT()])]), id = 43
LogicalTableScan(table=[[schema, TRANSACTIONS]]), id = 1
The failing logical plan only differs in the use of SUM instead of SUM0:
LogicalProject(AVG_SPEND_PM=[/(CASE(>($3, 0), $4, null:DOUBLE), $3)],
TRANS_FREQ_24=[$5]), id = 45
LogicalWindow(window#0=[window(partition {0} order by [2] range between
$3 PRECEDING and $4 PRECEDING aggs [COUNT($1), SUM($1)])],
window#1=[window(partition {0} order by [2] range between $5 PRECEDING and $4
PRECEDING aggs [COUNT()])]), id = 43
LogicalTableScan(table=[[schema, TRANSACTIONS]]), id = 1
But what you cannot see in the plans is the type of the row produced by the
logical window. The failing plan says that field $4 of the LogicalWindow is not
nullable, whereas it should nullable - in fact, the final result should be null
for a any 1-row.
I guess SUM0 never produces a NULL result, and that's the difference.
Maybe your change actually uncovers a different bug in the window
implementation.
> SqlToRelConverter should not replace windowed SUM with equivalent expression
> using SUM0
> ---------------------------------------------------------------------------------------
>
> Key: CALCITE-6020
> URL: https://issues.apache.org/jira/browse/CALCITE-6020
> Project: Calcite
> Issue Type: Improvement
> Reporter: Zoltan Haindrich
> Assignee: Norman Jordan
> Priority: Major
> Labels: pull-request-available
> Fix For: 1.38.0
>
>
> {{SqlToRelConverter}} replaces {{SUM}} with {{SUM0}} around
> [here|https://github.com/apache/calcite/blob/e1991e08a225ef08c2402ab35c310d88fff3c222/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java#L5885]
> This might have been needed at some point in the past - but I think it will
> be better to leave it as {{SUM}} - as in case there is no {{SUM0}} in the
> system that will be replaced with a {{COALESCE(SUM(...) , 0 )}} to provide it
> - as see
> [here|https://github.com/apache/calcite/blob/e1991e08a225ef08c2402ab35c310d88fff3c222/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java#L1288]
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
