[
https://issues.apache.org/jira/browse/CALCITE-4773?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17415530#comment-17415530
]
Ruben Q L edited comment on CALCITE-4773 at 9/15/21, 2:23 PM:
--------------------------------------------------------------
[~julianhyde], sorry to insist, but it seems to me that the logical plan (and
rowTypes) in this case are correct.
We are dealing with this plan:
{noformat}
LogicalAggregate(group=[{}], agg#0=[SINGLE_VALUE($0)])
LogicalProject(EXPR$0=[>($0, 0)])
LogicalAggregate(group=[{}], agg#0=[COUNT()])
LogicalFilter(condition=[=($0, 4)])
LogicalProject(VAL=[$0])
Uncollect
LogicalProject(Y=[$cor1.Y])
LogicalValues(tuples=[[{ 0 }]])
RowTypes:
LogicalAggregate(SINGLE_VALUE): RecordType(BOOLEAN NOT NULL $f0) NOT NULL
LogicalProject: RecordType(BOOLEAN NOT NULL EXPR$0) NOT NULL
LogicalAggregate(COUNT): RecordType(BIGINT NOT NULL $f0) NOT NULL
...
{noformat}
The return type of the count is bigint *non-nullable* (see
{{SqlCountAggFunction}}); which makes sense, since a COUNT can never return
null (it can return zero, but not null), is that correct? Thus, the return type
of the projection ($0 > 0) will consequently be a boolean non-null; and finally
the single_value function has a return type inference based on its argument
(see {{SqlSingleValueAggFunction}}), so we get the top boolean non-null.
Therefore, it would seem that the current precondition in {{RelDecorrelator}}'s
{{RemoveSingleAggregateRule#onMatch}}: _"singleAggRel produces a nullable
type..."_ has a loophole, and it can (in certain cases, at least this one) to
produce a non-nullable type, so the proposed PR could be a valid solution for
the issue.
was (Author: rubenql):
[~julianhyde], sorry to insist, but it seems to me that the logical plan (and
rowTypes) in this case are correct.
We are dealing with an Aggregate that contains as agg function a COUNT:
{noformat}
> expression is LogicalProject($f0=[CAST(>($0, 0)):BOOLEAN])
> LogicalAggregate(group=[{}], agg#0=[COUNT()])
> LogicalFilter(condition=[=($0, 4)])
> LogicalProject(VAL=[$0])
> Uncollect
> LogicalProject(Y=[$cor1.Y])
> LogicalValues(tuples=[[{ 0 }]])
{noformat}
and the return type of the count is BIGINT *non-nullable* (see
{{SqlCountAggFunction}}); which makes sense, since a COUNT can never return
null (it can return zero, but not null), is that correct?
Therefore, it would seem that the current precondition in
{{RelDecorrelator}}'s {{RemoveSingleAggregateRule#onMatch}}: _"singleAggRel
produces a nullable type..."_ has a loophole, and it can (in certain cases, at
least this one) to produce a non-nullable type, so the proposed PR could be a
valid solution for the issue.
> RelDecorrelator's RemoveSingleAggregateRule can produce result with wrong row
> type
> ----------------------------------------------------------------------------------
>
> Key: CALCITE-4773
> URL: https://issues.apache.org/jira/browse/CALCITE-4773
> Project: Calcite
> Issue Type: Bug
> Components: core
> Affects Versions: 1.27.0
> Reporter: Ruben Q L
> Assignee: Ruben Q L
> Priority: Major
> Labels: pull-request-available
> Fix For: 1.28.0
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> The problem can be reproduced with the following test (to be added to
> {{unnest.iq}}):
> {noformat}
> SELECT unnested_outer.val, COUNT(1) AS count_val
> FROM
> (
> SELECT *
> FROM (VALUES (1, array [3, 5]), (2, array [6, 4, 6]))
> AS u(x, y)
> ) AS t, UNNEST(t.y) AS unnested_outer(val)
> WHERE
> (
> SELECT COUNT(unnested_inner.val) > 0
> FROM UNNEST(t.y) AS unnested_inner(val)
> WHERE unnested_inner.val = 4
> )
> GROUP BY unnested_outer.val
> ORDER BY count_val DESC, unnested_outer.val ASC;
> +-----+-----------+
> | VAL | COUNT_VAL |
> +-----+-----------+
> | 6 | 2 |
> | 4 | 1 |
> +-----+-----------+
> (2 rows)
> !ok
> {noformat}
> When the test is executed, it fails with the following error:
> {noformat}
> > java.lang.AssertionError: Cannot add expression of different type to set:
> > set type is RecordType(BOOLEAN NOT NULL $f0) NOT NULL
> > expression type is RecordType(BOOLEAN $f0) NOT NULL
> > set is
> > rel#24436:LogicalAggregate.NONE.[](input=HepRelVertex#24435,group={},agg#0=SINGLE_VALUE($0))
> > expression is LogicalProject($f0=[CAST(>($0, 0)):BOOLEAN])
> > LogicalAggregate(group=[{}], agg#0=[COUNT()])
> > LogicalFilter(condition=[=($0, 4)])
> > LogicalProject(VAL=[$0])
> > Uncollect
> > LogicalProject(Y=[$cor1.Y])
> > LogicalValues(tuples=[[{ 0 }]])
> 249a254,325
> > at
> > org.apache.calcite.plan.RelOptUtil.verifyTypeEquivalence(RelOptUtil.java:391)
> > at
> > org.apache.calcite.plan.hep.HepRuleCall.transformTo(HepRuleCall.java:60)
> > at
> > org.apache.calcite.plan.RelOptRuleCall.transformTo(RelOptRuleCall.java:269)
> > at
> > org.apache.calcite.plan.RelOptRuleCall.transformTo(RelOptRuleCall.java:284)
> > at
> > org.apache.calcite.sql2rel.RelDecorrelator$RemoveSingleAggregateRule.onMatch(RelDecorrelator.java:1923)
> > at
> > org.apache.calcite.plan.AbstractRelOptPlanner.fireRule(AbstractRelOptPlanner.java:341)
> > at org.apache.calcite.plan.hep.HepPlanner.applyRule(HepPlanner.java:565)
> > at
> > org.apache.calcite.plan.hep.HepPlanner.applyRules(HepPlanner.java:428)
> > at
> > org.apache.calcite.plan.hep.HepPlanner.executeInstruction(HepPlanner.java:251)
> > at
> > org.apache.calcite.plan.hep.HepInstruction$RuleInstance.execute(HepInstruction.java:130)
> > at
> > org.apache.calcite.plan.hep.HepPlanner.executeProgram(HepPlanner.java:208)
> > at
> > org.apache.calcite.plan.hep.HepPlanner.findBestExp(HepPlanner.java:195)
> > at
> > org.apache.calcite.sql2rel.RelDecorrelator.removeCorrelationViaRule(RelDecorrelator.java:378)
> > at
> > org.apache.calcite.sql2rel.RelDecorrelator.decorrelateQuery(RelDecorrelator.java:221)
> ...
> {noformat}
> The root cause seems to be in this piece of code inside {{RelDecorrelator}}'s
> {{RemoveSingleAggregateRule#onMatch}}:
> {code:java}
> // singleAggRel produces a nullable type, so create the new
> // projection that casts proj expr to a nullable type.
> final RelBuilder relBuilder = call.builder();
> final RelDataType type =
> relBuilder.getTypeFactory()
> .createTypeWithNullability(projExprs.get(0).getType(), true);
> final RexNode cast = relBuilder.getRexBuilder().makeCast(type,
> projExprs.get(0));
> relBuilder.push(aggregate).project(cast);
> call.transformTo(relBuilder.build());
> {code}
> Note that the comment assumes that _"singleAggRel produces a nullable type"_,
> but in this particular case, it seems to produce a non-nullable type, so
> probably this piece of code needs to be adapted.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
