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 4d39ec06d2e2f26286ae7a6f21fb9d3f7ea85bff Author: Julian Hyde <[email protected]> AuthorDate: Wed Mar 23 15:46:12 2022 -0700 amend [CALCITE-5044] JDBC adapter generates integer literal in ORDER BY, which some dialects wrongly interpret as a reference to a field When an ORDER BY field turns out to be an integer literal, if the target dialect treats ordinals as field references, RelToRelConverter now generates a character literal instead. All constants have no effect on the sort, and therefore the character literal has the desired effect. Close apache/calcite#2749 --- .../apache/calcite/rel/rel2sql/SqlImplementor.java | 22 ++- .../calcite/rel/rel2sql/RelToSqlConverterTest.java | 157 +++++++++------------ 2 files changed, 76 insertions(+), 103 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java b/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java index 7edc42a..36cb305 100644 --- a/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java +++ b/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java @@ -608,7 +608,15 @@ public abstract class SqlImplementor { * alias, switches to a qualified column reference. */ public SqlNode orderField(int ordinal) { - return field(ordinal); + final SqlNode node = field(ordinal); + if (node instanceof SqlNumericLiteral + && dialect.getConformance().isSortByOrdinal()) { + // An integer literal will be wrongly interpreted as a field ordinal. + // Convert it to a character literal, which will have the same effect. + final String strValue = ((SqlNumericLiteral) node).toValue(); + return SqlLiteral.createCharString(strValue, node.getParserPosition()); + } + return node; } /** Converts an expression from {@link RexNode} to {@link SqlNode} @@ -1104,16 +1112,6 @@ public abstract class SqlImplementor { } void addOrderItem(List<SqlNode> orderByList, RelFieldCollation field) { - if (dialect.getConformance().isSortByOrdinal()) { - final SqlNode orderByNode = field(field.getFieldIndex()); - if (orderByNode instanceof SqlNumericLiteral) { - // We rewrite the current node for StringLiteral, when it's numeric constant. - final SqlNumericLiteral numericLiteralOrderBy = (SqlNumericLiteral) orderByNode; - final String strValue = numericLiteralOrderBy.toValue(); - orderByList.add(SqlLiteral.createCharString(strValue, orderByNode.getParserPosition())); - return; - } - } if (field.nullDirection != RelFieldCollation.NullDirection.UNSPECIFIED) { final boolean first = field.nullDirection == RelFieldCollation.NullDirection.FIRST; @@ -1739,7 +1737,7 @@ public abstract class SqlImplementor { // SELECT deptno AS empno, empno AS x FROM emp ORDER BY 2 // "ORDER BY empno" would give incorrect result; // "ORDER BY x" is acceptable but is not preferred. - final SqlNode node = field(ordinal); + final SqlNode node = super.orderField(ordinal); if (node instanceof SqlIdentifier && ((SqlIdentifier) node).isSimple()) { final String name = ((SqlIdentifier) node).getSimple(); diff --git a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java index bd9e434..24ee3a9 100644 --- a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java +++ b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java @@ -23,7 +23,6 @@ import org.apache.calcite.plan.RelTraitDef; import org.apache.calcite.plan.hep.HepPlanner; import org.apache.calcite.plan.hep.HepProgramBuilder; import org.apache.calcite.rel.RelCollations; -import org.apache.calcite.rel.RelFieldCollation; import org.apache.calcite.rel.RelNode; import org.apache.calcite.rel.core.JoinRelType; import org.apache.calcite.rel.hint.HintPredicates; @@ -41,6 +40,7 @@ import org.apache.calcite.rel.type.RelDataType; import org.apache.calcite.rel.type.RelDataTypeFactory; import org.apache.calcite.rel.type.RelDataTypeSystem; import org.apache.calcite.rel.type.RelDataTypeSystemImpl; +import org.apache.calcite.rex.RexNode; import org.apache.calcite.runtime.FlatLists; import org.apache.calcite.runtime.Hook; import org.apache.calcite.schema.SchemaPlus; @@ -82,6 +82,7 @@ import org.apache.calcite.tools.RelBuilder; import org.apache.calcite.tools.RuleSet; import org.apache.calcite.tools.RuleSets; import org.apache.calcite.util.ImmutableBitSet; +import org.apache.calcite.util.ImmutableIntList; import org.apache.calcite.util.TestUtil; import org.apache.calcite.util.Util; @@ -90,6 +91,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import org.checkerframework.checker.nullness.qual.Nullable; +import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.Test; import java.util.Collection; @@ -1598,109 +1600,82 @@ class RelToSqlConverterTest { sql(query).ok(expected); } - @Test void testRewriteOrderByWithAllConstants() { - final Function<RelBuilder, RelNode> relFn = b -> b - .scan("EMP") - .project(b.literal(1), b.field(1), b.field(2)) - .sort( - RelCollations.of( - ImmutableList.of( - new RelFieldCollation(0)))) - .project(b.field(2), b.field(1)) - .build(); - // Default dialect rewrite numeric constant keys to string literal in the order-by. - relFn(relFn).ok("SELECT \"JOB\", \"ENAME\"\n" - + "FROM \"scott\".\"EMP\"\n" - + "ORDER BY '1'"); - // Define a tested dialect doesn't remove constant keys in the order-by. - final SqlDialect testDialect = new SqlDialect(SqlDialect.EMPTY_CONTEXT) { - @Override - public SqlConformance getConformance() { + /** A dialect that doesn't treat integer literals in the ORDER BY as field + * references. */ + private SqlDialect nonOrdinalDialect() { + return new SqlDialect(SqlDialect.EMPTY_CONTEXT) { + @Override public SqlConformance getConformance() { return SqlConformanceEnum.STRICT_99; } }; - relFn(relFn).dialect(testDialect).ok("SELECT JOB, ENAME\n" - + "FROM scott.EMP\n" - + "ORDER BY 1"); } - @Test void testRewriteOrderByWithNotAllConstants() { - final Function<RelBuilder, RelNode> relFn = b -> b - .scan("EMP") - .project(b.literal(1), b.field(1), b.field(2)) - .sort( - RelCollations.of( - ImmutableList.of( - new RelFieldCollation(0), new RelFieldCollation(1)))) + @NotNull private RelNode scanProjectSort(RelBuilder b, RexNode node, + ImmutableIntList sortFields) { + return b.scan("EMP") + .project(node, b.field(1), b.field(2)) + .sort(RelCollations.of(sortFields)) .project(b.field(2), b.field(1)) .build(); - // Default dialect rewrite numeric constant keys to string literal in the order-by. - relFn(relFn).ok("SELECT \"JOB\", \"ENAME\"\n" - + "FROM \"scott\".\"EMP\"\n" - + "ORDER BY '1', \"ENAME\""); - // Define a tested dialect doesn't remove constant keys in the order-by. - final SqlDialect testDialect = new SqlDialect(SqlDialect.EMPTY_CONTEXT) { - @Override - public SqlConformance getConformance() { - return SqlConformanceEnum.STRICT_99; - } - }; - relFn(relFn).dialect(testDialect).ok("SELECT JOB, ENAME\n" - + "FROM scott.EMP\n" - + "ORDER BY 1, ENAME"); + } + + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-5044";>[CALCITE-5044] + * JDBC adapter generates integer literal in ORDER BY, which some dialects + * wrongly interpret as a reference to a field</a>. */ + @Test void testRewriteOrderByWithAllConstants() { + // Default dialect rewrite numeric constant keys to string literal in the + // ORDER BY. + relFn(b -> scanProjectSort(b, b.literal(1), ImmutableIntList.of(0))) + .ok("SELECT \"JOB\", \"ENAME\"\n" + + "FROM \"scott\".\"EMP\"\n" + + "ORDER BY '1'") + .dialect(nonOrdinalDialect()) + .ok("SELECT JOB, ENAME\n" + + "FROM scott.EMP\n" + + "ORDER BY 1"); + } + + @Test void testRewriteOrderByWithNotAllConstants() { + // Default dialect rewrite numeric constant keys to string literal in the + // ORDER BY. + relFn(b -> scanProjectSort(b, b.literal(1), ImmutableIntList.of(0, 1))) + .ok("SELECT \"JOB\", \"ENAME\"\n" + + "FROM \"scott\".\"EMP\"\n" + + "ORDER BY '1', \"ENAME\"") + .dialect(nonOrdinalDialect()) + .ok("SELECT JOB, ENAME\n" + + "FROM scott.EMP\n" + + "ORDER BY 1, ENAME"); } @Test void testRewriteOrderByConstantsWithAlias() { - final Function<RelBuilder, RelNode> relFn = b -> b - .scan("EMP") - .project(b.alias(b.literal(1), "col"), b.field(1), b.field(2)) - .sort( - RelCollations.of( - ImmutableList.of( - new RelFieldCollation(0), new RelFieldCollation(1)))) - .project(b.field(2), b.field(1)) - .build(); - // Default dialect rewrite numeric constant keys to string literal in the order-by. - relFn(relFn).ok("SELECT \"JOB\", \"ENAME\"\n" - + "FROM \"scott\".\"EMP\"\n" - + "ORDER BY '1', \"ENAME\""); - // Define a tested dialect doesn't remove constant keys in the order-by. - final SqlDialect testDialect = new SqlDialect(SqlDialect.EMPTY_CONTEXT) { - @Override - public SqlConformance getConformance() { - return SqlConformanceEnum.STRICT_99; - } - }; - relFn(relFn).dialect(testDialect).ok("SELECT JOB, ENAME\n" - + "FROM scott.EMP\n" - + "ORDER BY 1, ENAME"); + // Default dialect rewrite numeric constant keys to string literal in the + // ORDER BY. + relFn(b -> + scanProjectSort(b, b.alias(b.literal(1), "col"), + ImmutableIntList.of(0, 1))) + .ok("SELECT \"JOB\", \"ENAME\"\n" + + "FROM \"scott\".\"EMP\"\n" + + "ORDER BY '1', \"ENAME\"") + .dialect(nonOrdinalDialect()) + .ok("SELECT JOB, ENAME\n" + + "FROM scott.EMP\n" + + "ORDER BY 1, ENAME"); } @Test void testNoNeedRewriteOrderByConstantsForOtherType() { - final Function<RelBuilder, RelNode> relFn = b -> b - .scan("EMP") - .project(b.literal("12"), b.field(1), b.field(2)) - .sort( - RelCollations.of( - ImmutableList.of( - new RelFieldCollation(0), new RelFieldCollation(1)))) - .project(b.field(2), b.field(1)) - .build(); - // Default dialect rewrite numeric constant keys to string literal in the order-by, - // and string constant will not be written. - relFn(relFn).ok("SELECT \"JOB\", \"ENAME\"\n" - + "FROM \"scott\".\"EMP\"\n" - + "ORDER BY '12', \"ENAME\""); - // Define a tested dialect doesn't remove constant keys in the order-by. - final SqlDialect testDialect = new SqlDialect(SqlDialect.EMPTY_CONTEXT) { - @Override - public SqlConformance getConformance() { - return SqlConformanceEnum.STRICT_99; - } - }; - relFn(relFn).dialect(testDialect).ok("SELECT JOB, ENAME\n" - + "FROM scott.EMP\n" - + "ORDER BY '12', ENAME"); + // Default dialect rewrite numeric constant keys to string literal in the + // ORDER BY, and string constant will not be written. + relFn(b -> + scanProjectSort(b, b.literal("12"), ImmutableIntList.of(0, 1))) + .ok("SELECT \"JOB\", \"ENAME\"\n" + + "FROM \"scott\".\"EMP\"\n" + + "ORDER BY '12', \"ENAME\"") + .dialect(nonOrdinalDialect()) + .ok("SELECT JOB, ENAME\n" + + "FROM scott.EMP\n" + + "ORDER BY '12', ENAME"); } @Test void testNoNeedRewriteOrderByConstantsForOver() {
