Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CALCITE-5530] RelToSqlConverter[ORDER BY] generates an incorrect fie… #3085

Closed
wants to merge 7 commits into from
Closed
Original file line number Diff line number Diff line change
Expand Up @@ -1751,7 +1751,7 @@ private Builder builder(RelNode rel, Set<Clause> clauses) {
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);
}
Expand Down Expand Up @@ -1818,9 +1818,10 @@ private boolean needNewSubQuery(

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) {
Expand Down Expand Up @@ -1855,7 +1856,7 @@ private boolean hasSortByOrdinal(@UnknownInitialization Result this) {
}
for (SqlNode sqlNode : orderList) {
if (!(sqlNode instanceof SqlBasicCall)) {
return false;
return sqlNode instanceof SqlNumericLiteral;
}
for (SqlNode operand : ((SqlBasicCall) sqlNode).getOperandList()) {
if (operand instanceof SqlNumericLiteral) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 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"
Expand Down