Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/23516 )
Change subject: IMPALA-14405: Labels for Calcite expressions not matching original planner ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/23516/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteRelNodeConverter.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteRelNodeConverter.java: http://gerrit.cloudera.org:8080/#/c/23516/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteRelNodeConverter.java@179 PS2, Line 179: if (fieldName.startsWith("EXPR$")) { > Added a test with duplicate expressions. Done http://gerrit.cloudera.org:8080/#/c/23516/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteRelNodeConverter.java@184 PS2, Line 184: fieldName = selectItem.toSqlString(PostgresqlSqlDialect.DEFAULT).getSql(); : } : fieldNamesBuilder.add(fieldName.toLowerCase( > Yeah, this caused a bit of concern for me. This is a hard thing to test, be I ran a couple of simple tests with an online Postgres and MySQL database to compare the field names. Pasting it here: Postgres: select 1+1, 1+1; ?column? | ?column? ----------+---------- 2 | 2 select LENGTH('Hello'), Length('Hello') as Length1, LENGTH('Hello') as "Length2"; length | length1 | Length2 --------+---------+--------- 5 | 5 | 5 MySQL: select 1+1, 1+1; +-----+-----+ | 1+1 | 1+1 | +-----+-----+ | 2 | 2 | +-----+-----+ select LENGTH('Hello'), Length('Hello') as Length1, LENGTH('Hello') as "Length2"; +-----------------+---------+---------+ | LENGTH('Hello') | Length1 | Length2 | +-----------------+---------+---------+ | 5 | 5 | 5 | +-----------------+---------+---------+ For expression such as 1+1, Postgres uses a generic '?column?' alias whereas MySQL uses the expression itself as the label. For functions, Postgres takes only the function name. not its argument and it lowercases the name and the alias unless the alias is enclosed in double quotes. Whereas MySQL prints both the function name and its argument but it does preserve the case i.e it does not convert to lowercase. This is a small sample but based on this, although Impala behavior doesn't match either Postgres or Mysql, it is closer to MySQL (the difference being that MySQL preserves the case of the function name). It would be useful to see if changing to MySQL dialect causes any test failures. If not, I think it is better to use MySQL. http://gerrit.cloudera.org:8080/#/c/23516/3/testdata/workloads/functional-query/queries/QueryTest/calcite.test File testdata/workloads/functional-query/queries/QueryTest/calcite.test: http://gerrit.cloudera.org:8080/#/c/23516/3/testdata/workloads/functional-query/queries/QueryTest/calcite.test@1082 PS3, Line 1082: select 1; nit: This test seems redundant since the next one is a superset of this test. Instead, it would be useful to have an expression such as 1+1 be tested. -- To view, visit http://gerrit.cloudera.org:8080/23516 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibd3e6366a284f53807b4b2c42efafa279249c1ea Gerrit-Change-Number: 23516 Gerrit-PatchSet: 3 Gerrit-Owner: Steve Carlin <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Comment-Date: Sat, 18 Oct 2025 00:27:27 +0000 Gerrit-HasComments: Yes
