Skip to content
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

[CALCITE-6834] In query that applies COALESCE to nullable SUM, Enumer… #4201

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
import org.apache.calcite.util.mapping.MappingType;
import org.apache.calcite.util.mapping.Mappings;

import com.google.common.collect.ImmutableList;

import org.immutables.value.Value;

import java.math.BigDecimal;
Expand Down Expand Up @@ -154,8 +156,22 @@ && kindCount(project.getProjects(), SqlKind.CASE) == 0) {
builder.push(aggregate.getInput());
builder.aggregate(
builder.groupKey(aggregate.getGroupSet(), aggregate.groupSets), aggCallList);
builder.project(
RexPermuteInputsShuttle.of(mapping).visitList(projects2));
// If the output type of Aggregate does not match the input type required by original Project,
// CAST needs to be added
List<RexNode> rexInputRefs = RexPermuteInputsShuttle.of(mapping).visitList(projects2);
final ImmutableList.Builder<RexNode> selectList = ImmutableList.builder();
final RexShuttle rexShuttle = new RexShuttle() {
@Override public RexNode visitInputRef(RexInputRef inputRef) {
return builder.getRexBuilder().ensureType(
inputRef.getType(),
builder.fields().get(inputRef.getIndex()),
true);
}
};
for (RexNode rexInputRef : rexInputRefs) {
selectList.add(rexInputRef.accept(rexShuttle));
}
builder.project(selectList.build());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can achieve the same effect with one line of code: use RelBuilder.cast to convert the output row type of aggregate to what project expects it to be. If cast adds a CAST call then project will flatten the expression on top of it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What you are referring to here should be RelBuilder.convert, which also requires us to build a new RowType based on Mapping, so I have not made any changes here.

call.transformTo(builder.build());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6925,6 +6925,13 @@ private HepProgram getTransitiveProgram() {
.check();
}

@Test void testProjectAggregateMergeSum01() {
final String sql = "select coalesce(sum(cast(mgr as tinyint)), 0) as ss0\n"
+ "from sales.emp";
sql(sql).withRule(CoreRules.PROJECT_AGGREGATE_MERGE)
.check();
}

/** As {@link #testProjectAggregateMergeSum0()} but there is another use of
* {@code SUM} that cannot be converted to {@code SUM0}. */
@Test void testProjectAggregateMergeSum0AndSum() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8126,6 +8126,28 @@ LogicalProject(SS0=[CASE(IS NOT NULL($0), CAST($0):INTEGER NOT NULL, 0)])
LogicalAggregate(group=[{}], agg#0=[$SUM0($0)])
LogicalProject(SAL=[$5])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
</Resource>
</TestCase>
<TestCase name="testProjectAggregateMergeSum01">
<Resource name="sql">
<![CDATA[select coalesce(sum(cast(mgr as tinyint)), 0) as ss0
from sales.emp]]>
</Resource>
<Resource name="planBefore">
<![CDATA[
LogicalProject(SS0=[CASE(IS NOT NULL($0), CAST($0):INTEGER NOT NULL, 0)])
LogicalAggregate(group=[{}], agg#0=[SUM($0)])
LogicalProject($f0=[CAST($3):TINYINT])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
</Resource>
<Resource name="planAfter">
<![CDATA[
LogicalProject($f0=[CAST($0):INTEGER NOT NULL])
LogicalAggregate(group=[{}], agg#0=[$SUM0($0)])
LogicalProject($f0=[CAST($3):TINYINT])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
</Resource>
</TestCase>
Expand Down
47 changes: 47 additions & 0 deletions core/src/test/resources/sql/agg.iq
Original file line number Diff line number Diff line change
Expand Up @@ -3815,4 +3815,51 @@ select distinct sum(deptno + '1') as deptsum from dept order by 1;

!ok

# [CALCITE-2192] In query that applies COALESCE to nullable SUM, EnumerableProjectToCalcRule throws AssertionError
# sum(mgr) return TINYINT, coalesce(sum(mgr),0) return INTEGER, so need CAST
select coalesce(sum(mgr),0) as m
from emp;
java.sql.SQLException: Error while executing SQL "select coalesce(sum(mgr),0) as m
from emp": Value 38835 out of range
!error
EnumerableCalc(expr#0=[{inputs}], expr#1=[CAST($t0):INTEGER NOT NULL], $f0=[$t1])
EnumerableAggregate(group=[{}], agg#0=[$SUM0($3)])
EnumerableTableScan(table=[[scott, EMP]])
!plan

# sum(cast(mgr as int)) return INTEGER, coalesce(sum(cast(mgr as int)),0) return INTEGER, so don't need CAST
select coalesce(sum(cast(mgr as int)),0) as m
from emp;
+--------+
| M |
+--------+
| 100611 |
+--------+
(1 row)

!ok

EnumerableAggregate(group=[{}], agg#0=[$SUM0($0)])
EnumerableCalc(expr#0..7=[{inputs}], expr#8=[CAST($t3):INTEGER], $f0=[$t8])
EnumerableTableScan(table=[[scott, EMP]])
!plan

# sum(mgr) return INTEGER, coalesce(sum(mgr),0) return BIGINT, so need CAST
select coalesce(sum(cast(mgr as int)),cast(0 as bigint)) as m
from emp;
+--------+
| M |
+--------+
| 100611 |
+--------+
(1 row)

!ok

EnumerableCalc(expr#0=[{inputs}], expr#1=[CAST($t0):BIGINT NOT NULL], $f0=[$t1])
EnumerableAggregate(group=[{}], agg#0=[$SUM0($0)])
EnumerableCalc(expr#0..7=[{inputs}], expr#8=[CAST($t3):INTEGER], $f0=[$t8])
EnumerableTableScan(table=[[scott, EMP]])
!plan

# End agg.iq