Steve Carlin 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 4:

(2 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@184
PS2, Line 184:        // to grab the expression as/is to use for the label.
             :         fieldName = 
selectItem.toSqlString(MysqlSqlDialect.DEFAULT).getSql();
             :       }
> I ran a couple of simple tests with an online Postgres and MySQL database t
There may be a little confusion here...

Although we are using this code for labels, the code here isn't using the 
dialect to try and match the label name.  So the labels returned by these 
databases are irrelevant.

Instead, it is taking the "selectItem" as/is, and using any random dialect to 
get that expression back out.  In our case here, the selectItem will contain "1 
+ 1", and the "getSql()" method will return 1 + 1 in all dialects.

Having said that, I still changed the dialect to 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 2, 1 + 1;
> nit: This test seems redundant since the next one is a superset of this tes
Combined the two tests and added 1 + 1



--
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: 4
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: Mon, 20 Oct 2025 18:35:25 +0000
Gerrit-HasComments: Yes

Reply via email to