libenchao commented on code in PR #3085:
URL: https://github.com/apache/calcite/pull/3085#discussion_r1182260546
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -879,6 +879,78 @@ private static String toSql(RelNode root, SqlDialect
dialect,
.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
testSortWithAnFieldNotInTheProjectionWithASameAliasAsThatInTheProjection() {
+ 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
testSortWithAnExpressionNotInTheProjectionThatRefersToUnderlyingFieldWithSameAlias()
{
Review Comment:
```suggestion
@Test void
testOrderByExpressionNotInTheProjectionThatRefersToUnderlyingFieldWithSameAlias()
{
```
##########
core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java:
##########
@@ -1791,6 +1791,15 @@ private boolean needNewSubQuery(
return false;
}
final Clause maxClause = Collections.max(clauses);
+
+ if (rel instanceof Project
Review Comment:
Is there any reason that you we need to move this block from L1819-L1825 to
here? If not, I would suggest to keep it there to make the change of git
history minimal and trackable.
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -879,6 +879,78 @@ private static String toSql(RelNode root, SqlDialect
dialect,
.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
testSortWithAnFieldNotInTheProjectionWithASameAliasAsThatInTheProjection() {
Review Comment:
```suggestion
@Test void
testOrderByFieldNotInTheProjectionWithASameAliasAsThatInTheProjection() {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
