This is an automated email from the ASF dual-hosted git repository.
jhyde pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/master by this push:
new a6a1e2c [CALCITE-5044] JDBC adapter generates integer literal in
ORDER BY, which some dialects wrongly interpret as a reference to a field
a6a1e2c is described below
commit a6a1e2cef332893fd90286098869c56529e052c3
Author: xurenhe <[email protected]>
AuthorDate: Tue Mar 15 19:06:28 2022 +0800
[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 | 12 ++++-
.../calcite/rel/rel2sql/RelToSqlConverterTest.java | 55 ++++++++++++++++++++++
2 files changed, 65 insertions(+), 2 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 1215a76..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}
@@ -1729,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 0523615..5860161 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
@@ -22,6 +22,10 @@ import org.apache.calcite.plan.RelOptRule;
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.RelFieldCollation.Direction;
+import org.apache.calcite.rel.RelFieldCollation.NullDirection;
import org.apache.calcite.rel.RelNode;
import org.apache.calcite.rel.core.JoinRelType;
import org.apache.calcite.rel.hint.HintPredicates;
@@ -66,6 +70,7 @@ import org.apache.calcite.sql.type.SqlTypeName;
import org.apache.calcite.sql.util.SqlOperatorTables;
import org.apache.calcite.sql.util.SqlShuttle;
import org.apache.calcite.sql.validate.SqlConformance;
+import org.apache.calcite.sql.validate.SqlConformanceEnum;
import org.apache.calcite.sql2rel.SqlToRelConverter;
import org.apache.calcite.test.CalciteAssert;
import org.apache.calcite.test.MockSqlOperatorTable;
@@ -1595,6 +1600,56 @@ class RelToSqlConverterTest {
sql(query).ok(expected);
}
+ /** 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;
+ }
+ };
+ }
+
+ /** 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 testRewriteOrderByWithNumericConstants() {
+ final Function<RelBuilder, RelNode> relFn = b -> b
+ .scan("EMP")
+ .project(b.literal(1), b.field(1), b.field(2), b.literal("23"),
+ b.alias(b.literal(12), "col1"), b.literal(34))
+ .sort(
+ RelCollations.of(
+ ImmutableList.of(
+ new RelFieldCollation(0), new RelFieldCollation(3), new
RelFieldCollation(4),
+ new RelFieldCollation(1),
+ new RelFieldCollation(5, Direction.DESCENDING,
NullDirection.LAST))))
+ .project(b.field(2), b.field(1))
+ .build();
+ // Default dialect rewrite numeric constant keys to string literal in the
order-by.
+ // case1: numeric constant - rewrite it.
+ // case2: string constant - no need rewrite it.
+ // case3: wrap alias to numeric constant - rewrite it.
+ // case4: wrap collation's info to numeric constant - rewrite it.
+ relFn(relFn)
+ .ok("SELECT \"JOB\", \"ENAME\"\n"
+ + "FROM \"scott\".\"EMP\"\n"
+ + "ORDER BY '1', '23', '12', \"ENAME\", '34' DESC NULLS LAST")
+ .dialect(nonOrdinalDialect())
+ .ok("SELECT JOB, ENAME\n"
+ + "FROM scott.EMP\n"
+ + "ORDER BY 1, '23', 12, ENAME, 34 DESC NULLS LAST");
+ }
+
+ @Test void testNoNeedRewriteOrderByConstantsForOver() {
+ final String query = "select row_number() over "
+ + "(order by 1 nulls last) from \"employee\"";
+ // Default dialect keep numeric constant keys in the over of order-by.
+ sql(query).ok("SELECT ROW_NUMBER() OVER (ORDER BY 1)\n"
+ + "FROM \"foodmart\".\"employee\"");
+ }
+
/** Test case for
* <a
href="https://issues.apache.org/jira/browse/CALCITE-3440";>[CALCITE-3440]
* RelToSqlConverter does not properly alias ambiguous ORDER BY</a>. */
