Xuebin Su has posted comments on this change. ( http://gerrit.cloudera.org:8080/24325 )
Change subject: IMPALA-15013: Fix NULL issues for PIVOT ...................................................................... Patch Set 3: (4 comments) > Patch Set 2: > > (5 comments) > > Thanks for fixing this! Thanks for your reviews! http://gerrit.cloudera.org:8080/#/c/24325/2/be/src/exprs/aggregate-functions.h File be/src/exprs/aggregate-functions.h: http://gerrit.cloudera.org:8080/#/c/24325/2/be/src/exprs/aggregate-functions.h@407 PS2, Line 407: static void AggIfInitZeroUpdate( > Nit, optional: Thanks! I would like the code added to follow immediately the related existing code to make reviewing easier. http://gerrit.cloudera.org:8080/#/c/24325/2/fe/src/main/java/org/apache/impala/analysis/PivotTableRef.java File fe/src/main/java/org/apache/impala/analysis/PivotTableRef.java: http://gerrit.cloudera.org:8080/#/c/24325/2/fe/src/main/java/org/apache/impala/analysis/PivotTableRef.java@331 PS2, Line 331: String transposeAggName = (aggName.equals("count") || aggName.equals("ndv")) ? > Second comment related to the one above: Thanks! No, because some aggregate functions, e.g. sample(), returns an empty string on an empty table. I don't think we can use aggif_init_zero() for such functions. http://gerrit.cloudera.org:8080/#/c/24325/2/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java: http://gerrit.cloudera.org:8080/#/c/24325/2/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@521 PS2, Line 521: " ndv_no_finalize(month) for month in (1 as v1)) as t;", > Related to my other comments... Thanks! The ndv_no_finalize() function is chosen only because it looks very similar to ndv(). This test case is to show that even such functions are not supported. http://gerrit.cloudera.org:8080/#/c/24325/2/testdata/workloads/functional-query/queries/QueryTest/pivot-clause.test File testdata/workloads/functional-query/queries/QueryTest/pivot-clause.test: http://gerrit.cloudera.org:8080/#/c/24325/2/testdata/workloads/functional-query/queries/QueryTest/pivot-clause.test@436 PS2, Line 436: # The PIVOT clause contains the NULL literal. > Do we want an NDV test too? Thanks! Added. -- To view, visit http://gerrit.cloudera.org:8080/24325 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idae4a510dd3d6c4cd16a3e03aa0161ad84863e5a Gerrit-Change-Number: 24325 Gerrit-PatchSet: 3 Gerrit-Owner: Xuebin Su <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Reviewer: Xuebin Su <[email protected]> Gerrit-Comment-Date: Wed, 20 May 2026 03:15:31 +0000 Gerrit-HasComments: Yes
