Gabor Kaszab has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21301 )

Change subject: IMPALA-13002: Iceberg V2 tables with Avro delete files aren't 
read properly
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21301/1/fe/src/main/java/org/apache/impala/catalog/IcebergDeleteTable.java
File fe/src/main/java/org/apache/impala/catalog/IcebergDeleteTable.java:

http://gerrit.cloudera.org:8080/#/c/21301/1/fe/src/main/java/org/apache/impala/catalog/IcebergDeleteTable.java@87
PS1, Line 87:       if (desc.hdfsTable.isSetAvroSchema()) {
I guess the issue is also true for AVRO equality delete files. Should we also 
add test coverage for that? (could be separate patch)


http://gerrit.cloudera.org:8080/#/c/21301/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-mixed-format-position-deletes.test
File 
testdata/workloads/functional-query/queries/QueryTest/iceberg-mixed-format-position-deletes.test:

http://gerrit.cloudera.org:8080/#/c/21301/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-mixed-format-position-deletes.test@92
PS1, Line 92: 
row_regex:'$NAMENODE/test-warehouse/$DATABASE.db/ice_mixed_formats_partitioned/data/j_trunc=2/.*-data-.*.orc','.*B','','.*'
there should be 2 ORC data files in the j_trunc=2, right? One for (2,2) and one 
for (3,3). You only check for 1 of such line.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iff13198991caf32c51cd9e0ace4454fd00216cf6
Gerrit-Change-Number: 21301
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <[email protected]>
Gerrit-Reviewer: Daniel Becker <[email protected]>
Gerrit-Reviewer: Gabor Kaszab <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Comment-Date: Mon, 22 Apr 2024 15:20:10 +0000
Gerrit-HasComments: Yes

Reply via email to