Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23705 )

Change subject: IMPALA-13756: Fix Iceberg V2 count(*) optimization for complex 
queries
......................................................................


Patch Set 3:

(5 comments)

Thanks for the comments!

http://gerrit.cloudera.org:8080/#/c/23705/2/common/thrift/Query.thrift
File common/thrift/Query.thrift:

http://gerrit.cloudera.org:8080/#/c/23705/2/common/thrift/Query.thrift@a991
PS2, Line 991:
             :
             :
> Can this be marked as deprecated instead?
I guess it can cause issues when coordinator and executor have different 
versions. Added a comment about the deprecation.


http://gerrit.cloudera.org:8080/#/c/23705/2/fe/src/main/java/org/apache/impala/analysis/IcebergV2CountStarAdjuster.java
File 
fe/src/main/java/org/apache/impala/analysis/IcebergV2CountStarAdjuster.java:

http://gerrit.cloudera.org:8080/#/c/23705/2/fe/src/main/java/org/apache/impala/analysis/IcebergV2CountStarAdjuster.java@20
PS2, Line 20:
> Add documentation about what this expression is about.
Done


http://gerrit.cloudera.org:8080/#/c/23705/2/testdata/workloads/functional-query/queries/QueryTest/iceberg-v2-count-star-optimization-in-complex-query.test
File 
testdata/workloads/functional-query/queries/QueryTest/iceberg-v2-count-star-optimization-in-complex-query.test:

http://gerrit.cloudera.org:8080/#/c/23705/2/testdata/workloads/functional-query/queries/QueryTest/iceberg-v2-count-star-optimization-in-complex-query.test@65
PS2, Line 65: ), table_b as (
            :     SELECT CO
> Add RUNTIME_PROFILE validation?
Done


http://gerrit.cloudera.org:8080/#/c/23705/2/testdata/workloads/functional-query/queries/QueryTest/iceberg-v2-count-star-optimization-in-complex-query.test@101
PS2, Line 101: --                                      this table has 3 rows in 
a data file without deletes, it
             : --
> nit: This result is a bit confusing to me if I compare it against the outpu
Contents of the tables should be the same, so diff counts should still be 0s. 
Added explanation to the output exprs.


http://gerrit.cloudera.org:8080/#/c/23705/2/testdata/workloads/functional-query/queries/QueryTest/iceberg-v2-count-star-optimization-in-complex-query.test@103
PS2, Line 103: --    "count(*)": count(*) in view 'diff_count'.
             : with diff_cou
> Can validate RUNTIME_PROFILE here, so test case below is not needed.
Done



--
To view, visit http://gerrit.cloudera.org:8080/23705
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1940031298eb634aa82c3d32bbbf16bce8eaf874
Gerrit-Change-Number: 23705
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Wed, 10 Dec 2025 11:49:18 +0000
Gerrit-HasComments: Yes

Reply via email to