This is an automated email from the ASF dual-hosted git repository. jhyde pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/calcite.git
commit b2f59d7b90deaca3575410437a6cc9c99bf2174d Author: Julian Hyde <[email protected]> AuthorDate: Tue Apr 11 00:50:27 2023 -0700 [CALCITE-5697] RelBuilder.convert does not match nullability if top of stack is a Project --- .../java/org/apache/calcite/rel/core/Project.java | 19 +++++++++++++++++++ .../main/java/org/apache/calcite/rex/RexUtil.java | 2 +- .../org/apache/calcite/sql2rel/RelDecorrelator.java | 20 +++++++++++--------- .../java/org/apache/calcite/test/RelBuilderTest.java | 9 +++++++-- 4 files changed, 38 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/rel/core/Project.java b/core/src/main/java/org/apache/calcite/rel/core/Project.java index ddcb037db9..3c4acb5842 100644 --- a/core/src/main/java/org/apache/calcite/rel/core/Project.java +++ b/core/src/main/java/org/apache/calcite/rel/core/Project.java @@ -37,6 +37,7 @@ import org.apache.calcite.rex.RexOver; import org.apache.calcite.rex.RexShuttle; import org.apache.calcite.rex.RexUtil; import org.apache.calcite.sql.SqlExplainLevel; +import org.apache.calcite.tools.RelBuilder; import org.apache.calcite.util.Litmus; import org.apache.calcite.util.Pair; import org.apache.calcite.util.Permutation; @@ -215,6 +216,24 @@ public abstract class Project extends SingleRel implements Hintable { return Pair.zip(getProjects(), getRowType().getFieldNames()); } + /** Returns a list of project expressions, each of which is wrapped in a + * call to {@code AS} if its field name differs from the default. + * + * <p>This method has a similar effect to {@link #getNamedProjects()}, + * but the single list is easier to manage. + * + * @see org.apache.calcite.tools.RelBuilder#alias(RexNode, String) + */ + // TODO: move to RelBuilder? + // TODO: replace calls to getNamedProjects + public final List<RexNode> getAliasedProjects(RelBuilder b) { + final ImmutableList.Builder<RexNode> builder = ImmutableList.builder(); + Pair.forEach(exps, getRowType().getFieldList(), (e, f) -> { + builder.add(b.alias(e, f.getName())); + }); + return builder.build(); + } + @Override public ImmutableList<RelHint> getHints() { return hints; } diff --git a/core/src/main/java/org/apache/calcite/rex/RexUtil.java b/core/src/main/java/org/apache/calcite/rex/RexUtil.java index 02fb508f2b..1f4a99ff79 100644 --- a/core/src/main/java/org/apache/calcite/rex/RexUtil.java +++ b/core/src/main/java/org/apache/calcite/rex/RexUtil.java @@ -147,7 +147,7 @@ public class RexUtil { if (lhsType.equals(rhsType)) { castExps.add(rhsExp); } else { - castExps.add(rexBuilder.makeCast(lhsType, rhsExp)); + castExps.add(rexBuilder.makeCast(lhsType, rhsExp, true, false)); } } return castExps; diff --git a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java b/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java index 1a1507d217..d40f681799 100644 --- a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java +++ b/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java @@ -1860,26 +1860,32 @@ public class RelDecorrelator implements ReflectiveVisitor { /** * Rule to remove an Aggregate with SINGLE_VALUE. For cases like: * + * <pre>{@code * Aggregate(SINGLE_VALUE) * Project(single expression) * Aggregate + * }</pre> * - * For instance (subtree taken from TPCH query 17): + * <p>For instance, the following subtree from TPCH query 17: * + * <pre>{@code * LogicalAggregate(group=[{}], agg#0=[SINGLE_VALUE($0)]) * LogicalProject(EXPR$0=[*(0.2:DECIMAL(2, 1), $0)]) * LogicalAggregate(group=[{}], agg#0=[AVG($0)]) * LogicalProject(L_QUANTITY=[$4]) * LogicalFilter(condition=[=($1, $cor0.P_PARTKEY)]) * LogicalTableScan(table=[[TPCH_01, LINEITEM]]) + * }</pre> * - * Will be converted into: + * <p>will be converted into: * + * <pre>{@code * LogicalProject($f0=[*(0.2:DECIMAL(2, 1), $0)]) * LogicalAggregate(group=[{}], agg#0=[AVG($0)]) * LogicalProject(L_QUANTITY=[$4]) * LogicalFilter(condition=[=($1, $cor0.P_PARTKEY)]) * LogicalTableScan(table=[[TPCH_01, LINEITEM]]) + * }</pre> */ public static final class RemoveSingleAggregateRule extends RelRule<RemoveSingleAggregateRule.RemoveSingleAggregateRuleConfig> { @@ -1923,13 +1929,9 @@ public class RelDecorrelator implements ReflectiveVisitor { // ensure we keep the same type after removing the SINGLE_VALUE Aggregate final RelBuilder relBuilder = call.builder(); - final RelDataType singleAggType = - singleAggregate.getRowType().getFieldList().get(0).getType(); - final RexNode oldProjectExp = projExprs.get(0); - final RexNode newProjectExp = singleAggType.equals(oldProjectExp.getType()) - ? oldProjectExp - : relBuilder.getRexBuilder().makeCast(singleAggType, oldProjectExp); - relBuilder.push(aggregate).project(newProjectExp); + relBuilder.push(aggregate) + .project(project.getAliasedProjects(relBuilder)) + .convert(singleAggregate.getRowType(), false); call.transformTo(relBuilder.build()); } diff --git a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java index 92413b02d3..31c3afea37 100644 --- a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java +++ b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java @@ -1191,14 +1191,19 @@ public class RelBuilderTest { builder.getTypeFactory().builder() .add("a", SqlTypeName.BIGINT) .add("b", SqlTypeName.VARCHAR, 10) - .add("c", SqlTypeName.VARCHAR, 10) + .add("c", SqlTypeName.VARCHAR, 10).nullable(true) + .add("d", SqlTypeName.INTEGER).nullable(true) .build(); RelNode root = builder.scan("DEPT") + .projectPlus(builder.alias(builder.literal(2), "two")) .convert(rowType, false) .build(); final String expected = "" - + "LogicalProject(DEPTNO=[CAST($0):BIGINT NOT NULL], DNAME=[CAST($1):VARCHAR(10) NOT NULL], LOC=[CAST($2):VARCHAR(10) NOT NULL])\n" + + "LogicalProject(DEPTNO=[CAST($0):BIGINT NOT NULL], " + + "DNAME=[CAST($1):VARCHAR(10) NOT NULL], " + + "LOC=[CAST($2):VARCHAR(10)], " + + "two=[CAST(2):INTEGER])\n" + " LogicalTableScan(table=[[scott, DEPT]])\n"; assertThat(root, hasTree(expected)); }
