Daniel Becker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23251 )

Change subject: IMPALA-14261: Take 'impala.computeStatsSnapshotId' into account 
when deciding between Puffin and HMS stats
......................................................................


Patch Set 6:

(3 comments)

Thanks for the reviews!

http://gerrit.cloudera.org:8080/#/c/23251/4/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/IcebergTable.java:

http://gerrit.cloudera.org:8080/#/c/23251/4/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@903
PS4, Line 903:
> nit: line continuations need +4 spaces.
Done


http://gerrit.cloudera.org:8080/#/c/23251/4/fe/src/main/java/org/apache/impala/catalog/PuffinStatsLoader.java
File fe/src/main/java/org/apache/impala/catalog/PuffinStatsLoader.java:

http://gerrit.cloudera.org:8080/#/c/23251/4/fe/src/main/java/org/apache/impala/catalog/PuffinStatsLoader.java@109
PS4, Line 109:
> nit: extra '.'
Done


http://gerrit.cloudera.org:8080/#/c/23251/4/fe/src/main/java/org/apache/impala/catalog/PuffinStatsLoader.java@122
PS4, Line 122: // There is no snapshot in the table yet.
> Nice! Could you add a test for this edge case?
Thanks, good idea. I've investigated this a bit but it's not really possible to 
assign a Puffin file to a table with no snapshot: the Puffin file needs a 
snapshot id for which it is calculated. If it is -1 or missing, the Iceberg 
library does not return the Puffin file even if the metadata.json file contains 
a "statistics" segment with everything else set up. So I don't think it's worth 
trying to set up a test for it.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95a5b152dd504e94dea368a107d412e33f67930c
Gerrit-Change-Number: 23251
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker <[email protected]>
Gerrit-Reviewer: Daniel Becker <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Wed, 13 Aug 2025 14:48:08 +0000
Gerrit-HasComments: Yes

Reply via email to