Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24208 )
Change subject: IMPALA-14903: Calcite planner: Simplify code for string literals ...................................................................... Patch Set 8: (3 comments) http://gerrit.cloudera.org:8080/#/c/24208/8/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceOperandShuttle.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceOperandShuttle.java: http://gerrit.cloudera.org:8080/#/c/24208/8/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceOperandShuttle.java@51 PS8, Line 51: import java.math.BigDecimal; > nit: unused now Done http://gerrit.cloudera.org:8080/#/c/24208/8/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/RexLiteralConverter.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/RexLiteralConverter.java: http://gerrit.cloudera.org:8080/#/c/24208/8/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/RexLiteralConverter.java@99 PS8, Line 99: ImpalaTypeConverter.createImpalaType(rexLiteral.getType()), false); > Does this mean string literals could use CHAR/VARCHAR types now? IIRC, we h This code should only kick in if there is an explicit cast to CHAR or VARCHAR. By default, the code always uses STRING for any string literals. In the case of IN, the IN operator does not support CHAR, then there will be an implicit cast to STRING at some point, so this will not be a problem. http://gerrit.cloudera.org:8080/#/c/24208/8/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/24208/8/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteRelNodeConverter.java@161 PS8, Line 161: // simplify step until after coerce nodes is complete. > Do we still need the NO_SIMPLIFY RelBuilderFactory? Unfortunately, yes. I changed the comment accordingly. -- To view, visit http://gerrit.cloudera.org:8080/24208 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id8e61b2555afd81ef52f19431fdd1224d4039c00 Gerrit-Change-Number: 24208 Gerrit-PatchSet: 8 Gerrit-Owner: Steve Carlin <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Comment-Date: Sun, 07 Jun 2026 03:02:39 +0000 Gerrit-HasComments: Yes
