Arnab Karmakar has posted comments on this change. ( http://gerrit.cloudera.org:8080/24871 )
Change subject: IMPALA-15203: Add aggregate function support for UUID columns ...................................................................... Patch Set 4: (6 comments) Thanks for the comments! http://gerrit.cloudera.org:8080/#/c/24871/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24871/2//COMMIT_MSG@33 PS2, Line 33: - FE: Reject COMPUTE STATS on UUID columns and allow column whitelists > The change makes COMPUTE STATS on UUID columns silently working, but it doe Disabled COMPUTE STATS on tables with UUID columns or when mentioned explicitly `COMPUTE STATS tbl (uuid_col)`. `COMPUTE STATS tbl (non_uuid_col)` still works even if the table has UUID columns. http://gerrit.cloudera.org:8080/#/c/24871/2/be/src/exec/aggregator.cc File be/src/exec/aggregator.cc: http://gerrit.cloudera.org:8080/#/c/24871/2/be/src/exec/aggregator.cc@452 PS2, Line 452: dst_type.type != TYPE_FIXED_UDA_INTERMEDIATE > Could be: Done http://gerrit.cloudera.org:8080/#/c/24871/2/be/src/exprs/agg-fn-evaluator.cc File be/src/exprs/agg-fn-evaluator.cc: http://gerrit.cloudera.org:8080/#/c/24871/2/be/src/exprs/agg-fn-evaluator.cc@308 PS2, Line 308: if (type.type == TYPE_CHAR || type.type == TYPE_FIXED_UDA_INTERMEDIATE : || type.type == TYPE_UUID) { > This can leave the uninitilized for MinUuid/MaxUuid. Consider: Thanks for the help. Didnt pay attention to the interpreted path execution earlier. http://gerrit.cloudera.org:8080/#/c/24871/2/be/src/exprs/aggregate-functions-ir.cc File be/src/exprs/aggregate-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/24871/2/be/src/exprs/aggregate-functions-ir.cc@1325 PS2, Line 1325: if (src.is_null) return; > You could add: Done http://gerrit.cloudera.org:8080/#/c/24871/2/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java File fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java: http://gerrit.cloudera.org:8080/#/c/24871/2/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java@1115 PS2, Line 1115: String minMaxInit = t.isStringType() ? initNullString : initNull; > Aggif and case are still not registered for UUID, so planner rewrites over Registered aggif and case for UUID. http://gerrit.cloudera.org:8080/#/c/24871/2/testdata/workloads/functional-query/queries/QueryTest/iceberg-uuid-type.test File testdata/workloads/functional-query/queries/QueryTest/iceberg-uuid-type.test: http://gerrit.cloudera.org:8080/#/c/24871/2/testdata/workloads/functional-query/queries/QueryTest/iceberg-uuid-type.test@724 PS2, Line 724: ==== > Please add the following tests as well, and run them with disable_codegen=t Done -- To view, visit http://gerrit.cloudera.org:8080/24871 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2187495c8be5b95ecff3f458b6549916466e9c83 Gerrit-Change-Number: 24871 Gerrit-PatchSet: 4 Gerrit-Owner: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Thu, 24 Sep 2026 09:16:37 +0000 Gerrit-HasComments: Yes
