Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24247 )
Change subject: IMPALA-14814: Handle implicit casts properly for Calcite Planner ...................................................................... Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/24247/11/fe/src/main/java/org/apache/impala/analysis/CastExpr.java File fe/src/main/java/org/apache/impala/analysis/CastExpr.java: http://gerrit.cloudera.org:8080/#/c/24247/11/fe/src/main/java/org/apache/impala/analysis/CastExpr.java@94 PS11, Line 94: protected CastExpr(TypeDef targetTypeDef, Expr e, String format) { > The difference in behavior between these two constructors concerns me a bit I hear ya... Not sure this will alleviate your concerns (or perhaps add to it!) but the Calcite planner is mainly responsible for the analysis. This is why the AnalyzedCastExpr class was created, which overrides the "analyzeImpl()" method. The "analyze" and "analyzeImpl" methods are where the logic gets merged from the two different constructors, so I think we're ok here. Perhaps it is confusing though. Overridden classes tend to be, sigh. Maybe there's a way to get rid of the confusion through interfaces somehow, but that might be quite a refactor. -- To view, visit http://gerrit.cloudera.org:8080/24247 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d71357ac7f228404713f96884d9eff7f8eabe5c Gerrit-Change-Number: 24247 Gerrit-PatchSet: 11 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: Steve Carlin <[email protected]> Gerrit-Comment-Date: Thu, 14 May 2026 16:13:38 +0000 Gerrit-HasComments: Yes
