liuyao has posted comments on this change. ( http://gerrit.cloudera.org:8080/17604 )
Change subject: IMPALA-9763: builtin functions throw unknown exception ...................................................................... Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/17604/5/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java File fe/src/main/java/org/apache/impala/analysis/QueryStmt.java: http://gerrit.cloudera.org:8080/#/c/17604/5/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@274 PS5, Line 274: orderByElement.getExpr().collectAll( : Predicates.instanceOf(FunctionCallExpr.class), fnExprs); : } : substituteOrdinalsAndAliases(orderingExprs, "ORDER BY", analyzer); : : for (FunctionCallExpr fn: fnExprs) fn.getFnName().analyze(analyzer); > I'm not clear about this. You are collecting fnExprs from the > original 'orderByElements_' but not the cloned ones inside > 'orderingExprs'. And then analyze these original exprs. > > Do we require all FunctionName objects being analyzed now? Just > concerning that it will introduce a new requirement that may be > broken in many other places. Previously, the code here relied on shallow copy. When executing substituteOrdinalsAndAliases(orderingExprs, "ORDER BY", analyzer), the FunctionName in orderingExprs was analyzed and the FunctionName in orderByElements_ was also analyzed. After changing to a deep copy, if the FunctionName in orderByElements_ is not analyzed here, then NormalizeCountStarRule#apply will report an exception: NullPointorException, because the FunctionName in orderByElements_ has not been analyzed, origExpr.getFnName().getFunction() returns Is null(line 41). -- To view, visit http://gerrit.cloudera.org:8080/17604 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifd77f5a7cd9a674340770233fe26eb5dc26e666a Gerrit-Change-Number: 17604 Gerrit-PatchSet: 6 Gerrit-Owner: liuyao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: liuyao <[email protected]> Gerrit-Comment-Date: Wed, 23 Jun 2021 09:37:55 +0000 Gerrit-HasComments: Yes
