This is an automated email from the ASF dual-hosted git repository.
libenchao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new cb34e78c23 [CALCITE-5530] RelToSqlConverter[ORDER BY] generates an
incorrect field alias when 2 projection fields have the same name
cb34e78c23 is described below
commit cb34e78c2305663cd3e2da06acf4ba7ad797947e
Author: xiejiajun <[email protected]>
AuthorDate: Sun Feb 26 17:15:55 2023 +0800
[CALCITE-5530] RelToSqlConverter[ORDER BY] generates an incorrect field
alias when 2 projection fields have the same name
Close apache/calcite#3085
---
.../apache/calcite/rel/rel2sql/SqlImplementor.java | 9 +--
.../calcite/rel/rel2sql/RelToSqlConverterTest.java | 72 ++++++++++++++++++++++
2 files changed, 77 insertions(+), 4 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 6af0bd989e..69e6dac5e8 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
@@ -1758,7 +1758,7 @@ public abstract class SqlImplementor {
if (selectItem.i != ordinal) {
final @Nullable String alias =
SqlValidatorUtil.alias(selectItem.e);
- if (name.equalsIgnoreCase(alias)) {
+ if (name.equalsIgnoreCase(alias) &&
dialect.getConformance().isSortByAlias()) {
return SqlLiteral.createExactNumeric(
Integer.toString(ordinal + 1), SqlParserPos.ZERO);
}
@@ -1825,9 +1825,10 @@ public abstract class SqlImplementor {
if (rel instanceof Project
&& clauses.contains(Clause.ORDER_BY)
- && dialect.getConformance().isSortByOrdinal()) {
+ && dialect.getConformance().isSortByOrdinal()
+ && hasSortByOrdinal()) {
// Cannot merge a Project that contains sort by ordinal under it.
- return hasSortByOrdinal();
+ return true;
}
if (rel instanceof Aggregate) {
@@ -1862,7 +1863,7 @@ public abstract class SqlImplementor {
}
for (SqlNode sqlNode : orderList) {
if (!(sqlNode instanceof SqlBasicCall)) {
- return false;
+ return sqlNode instanceof SqlNumericLiteral;
}
for (SqlNode operand : ((SqlBasicCall) sqlNode).getOperandList()) {
if (operand instanceof SqlNumericLiteral) {
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 4d146ec5d2..1d7431b922 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
@@ -877,6 +877,78 @@ class RelToSqlConverterTest {
.withPresto().ok(expectedPresto);
}
+ /**
+ * Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-5530";>[CALCITE-5530]
+ * RelToSqlConverter[ORDER BY] generates an incorrect field alias
+ * when 2 projection fields have the same name</a>.
+ */
+ @Test void
testOrderByFieldNotInTheProjectionWithASameAliasAsThatInTheProjection() {
+ final RelBuilder builder = relBuilder();
+ final RelNode base = builder
+ .scan("EMP")
+ .project(
+ builder.alias(
+ builder.call(SqlStdOperatorTable.UPPER,
builder.field("ENAME")), "EMPNO"),
+ builder.field("EMPNO")
+ )
+ .sort(1)
+ .project(builder.field(0))
+ .build();
+
+ // The expected string should deliberately have a subquery to handle a
scenario in which
+ // the projection field has an alias with the same name as that of the
field used in the
+ // ORDER BY
+ String expectedSql1 = ""
+ + "SELECT \"EMPNO\"\n"
+ + "FROM (SELECT UPPER(\"ENAME\") AS \"EMPNO\", \"EMPNO\" AS
\"EMPNO0\"\n"
+ + "FROM \"scott\".\"EMP\"\n"
+ + "ORDER BY 2) AS \"t0\"";
+ String actualSql1 = toSql(base);
+ assertThat(actualSql1, isLinux(expectedSql1));
+
+ String actualSql2 = toSql(base, nonOrdinalDialect());
+ String expectedSql2 = "SELECT UPPER(ENAME) AS EMPNO\n"
+ + "FROM scott.EMP\n"
+ + "ORDER BY EMPNO";
+ assertThat(actualSql2, isLinux(expectedSql2));
+ }
+
+ @Test void
testOrderByExpressionNotInTheProjectionThatRefersToUnderlyingFieldWithSameAlias()
{
+ final RelBuilder builder = relBuilder();
+ final RelNode base = builder
+ .scan("EMP")
+ .project(
+ builder.alias(
+ builder.call(SqlStdOperatorTable.UPPER,
builder.field("ENAME")), "EMPNO"),
+ builder.call(
+ SqlStdOperatorTable.PLUS, builder.field("EMPNO"),
+ builder.literal(1)
+ )
+ )
+ .sort(1)
+ .project(builder.field(0))
+ .build();
+
+ // An output such as
+ // "SELECT UPPER(\"ENAME\") AS \"EMPNO\"\nFROM \"scott\".\"EMP\"\nORDER BY
\"EMPNO\" + 1"
+ // would be incorrect since the rel is sorting by the field \"EMPNO\" + 1
in which EMPNO
+ // refers to the physical column EMPNO and not the alias
+ String actualSql1 = toSql(base);
+ String expectedSql1 = ""
+ + "SELECT \"EMPNO\"\n"
+ + "FROM (SELECT UPPER(\"ENAME\") AS \"EMPNO\", \"EMPNO\" + 1 AS
\"$f1\"\n"
+ + "FROM \"scott\".\"EMP\"\n"
+ + "ORDER BY 2) AS \"t0\"";
+ assertThat(actualSql1, isLinux(expectedSql1));
+
+ String actualSql2 = toSql(base, nonOrdinalDialect());
+ String expectedSql2 = "SELECT UPPER(ENAME) AS EMPNO\n"
+ + "FROM scott.EMP\n"
+ + "ORDER BY EMPNO + 1";
+ assertThat(actualSql2, isLinux(expectedSql2));
+ }
+
@Test void testSelectQueryWithMinAggregateFunction() {
String query = "select min(\"net_weight\") from \"product\" group by
\"product_class_id\" ";
final String expected = "SELECT MIN(\"net_weight\")\n"
