You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2022/03/23 23:53:42 UTC
[calcite] 01/01: amend [CALCITE-5044] JDBC adapter generates integer literal in ORDER BY, which some dialects wrongly interpret as a reference to a field
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 <jh...@apache.org>
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() {