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 6:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/24871/5/be/src/exprs/agg-fn-evaluator.cc
File be/src/exprs/agg-fn-evaluator.cc:

http://gerrit.cloudera.org:8080/#/c/24871/5/be/src/exprs/agg-fn-evaluator.cc@308
PS5, Line 308:   if (type.type == TYPE_CHAR || type.type == 
TYPE_FIXED_UDA_INTERMEDIATE
> does this change alter the behavior of existing types TYPE_CHAR and TYPE_FI
Done


http://gerrit.cloudera.org:8080/#/c/24871/5/be/src/exprs/aggregate-functions-ir.cc
File be/src/exprs/aggregate-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/24871/5/be/src/exprs/aggregate-functions-ir.cc@1314
PS5, Line 1314: // TODO(IMPALA-15431): When src.len == dst->len, overwrite dst 
in place instead of
              : // Free() + CopyStringVal().
              : template<>
              : void AggregateFunctions::Max(FunctionContext* ctx, const 
StringVal& src, StringVal* dst) {
              :   if (src.is_null) return;
              :   if (dst->is_null ||
              :       StringValue::FromStringVal(src) > 
StringValue::FromStringVal(*dst)) {
              :     if (!dst->is_null) ctx->Free(dst->ptr);
              :     CopyStringVal(ctx, src, dst);
              :   }
              : }
              :
              : void AggregateFunctions::MinUuid(FunctionContext*, const 
StringVal& src, StringVal* dst) {
              :   if (src.is_null) return;
              :   DCHECK_EQ(src.len, UUID_BYTE_LEN);
              :   if (dst->is_null || memcmp(src.ptr, dst->ptr, UUID_BYTE_LEN) 
< 0) {
              :     memcpy(dst->ptr, src.ptr, UUID_BYTE_LEN);
              :     dst->len = UUID_BYTE_LEN;
              :
> I was thinking about why string and uuid functions are different at all, th
Done. Filed IMPALA-15431 and marked it as a todo.


http://gerrit.cloudera.org:8080/#/c/24871/5/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:

http://gerrit.cloudera.org:8080/#/c/24871/5/tests/query_test/test_iceberg.py@2722
PS5, Line 2722:   """Tests related to Iceberg DIRECTED distribution mode."""
              :
              :   @classmethod
> Can't we use this during dataload ( functional_schema_template.sql)? Curren
Done. The execution has also become quite fast now.



--
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: 6
Gerrit-Owner: Arnab Karmakar <[email protected]>
Gerrit-Reviewer: Arnab Karmakar <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Fri, 25 Sep 2026 10:16:28 +0000
Gerrit-HasComments: Yes

Reply via email to