Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/23560 )
Change subject: IMPALA-14421: Calcite planner: case statement returning wrong types for char, varchar ...................................................................... Patch Set 4: (4 comments) http://gerrit.cloudera.org:8080/#/c/23560/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/23560/4//COMMIT_MSG@28 PS4, Line 28: . > Add a testing section? Done http://gerrit.cloudera.org:8080/#/c/23560/4/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/23560/4/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceOperandShuttle.java@197 PS4, Line 197: !literal.isNull() > Did we account for other edge cases like empty string literal etc? This code gets hit by all literals. I've tested this through the Impala suite and it's gone through fine. Just to be sure, I just tested an empty string and that works too. So I think we're good here. I don't want to go overboard adding tests into calcite.test because eventually this code will be tested by jenkins across all tests. http://gerrit.cloudera.org:8080/#/c/23560/4/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionResolver.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionResolver.java: http://gerrit.cloudera.org:8080/#/c/23560/4/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionResolver.java@215 PS4, Line 215: ot > nit: not? Done http://gerrit.cloudera.org:8080/#/c/23560/4/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/RexCallConverter.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/RexCallConverter.java: http://gerrit.cloudera.org:8080/#/c/23560/4/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/RexCallConverter.java@216 PS4, Line 216: e == > Maybe we wanna use .equals() or something. Good catch, lol, rookie mistake by me. I think this might be safe right now. The default types are constant. The only place that we would need to worry about this is with decimal non-wild card types and char and varchar wild card types and I don't think any functions return those. However... Once we support UDFs, I think that changes. So I've made the change. Thanks! -- To view, visit http://gerrit.cloudera.org:8080/23560 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I82d657f4bfce432c458ee8198188dadf9f23f2ef Gerrit-Change-Number: 23560 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: Pranav Lodha <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Comment-Date: Wed, 12 Nov 2025 01:08:07 +0000 Gerrit-HasComments: Yes
