Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/20595 )
Change subject: IMPALA-11387: Introduce virtual column to expose Iceberg's file-level data sequence number ...................................................................... Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/20595/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20595/1//COMMIT_MSG@11 PS1, Line 11: From Iceberg comments on ContentFile class: > Could add a link to the Iceberg docs. Done http://gerrit.cloudera.org:8080/#/c/20595/1//COMMIT_MSG@17 PS1, Line 17: > Does this refer to the data sequence number or the file sequence number? Here it doesn't matter if it is data or file sequence number. The point is that with compaction the file seq num is updated while the data seq num remains. http://gerrit.cloudera.org:8080/#/c/20595/1/be/src/exec/file-metadata-utils.cc File be/src/exec/file-metadata-utils.cc: http://gerrit.cloudera.org:8080/#/c/20595/1/be/src/exec/file-metadata-utils.cc@72 PS1, Line 72: > Could be 'else if'. Done http://gerrit.cloudera.org:8080/#/c/20595/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-virtual-columns.test File testdata/workloads/functional-query/queries/QueryTest/iceberg-virtual-columns.test: http://gerrit.cloudera.org:8080/#/c/20595/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-virtual-columns.test@187 PS1, Line 187: ==== > Do you think it would be useful to add tests where we select the data seque It wouldn't hurt to add more coverage. -- To view, visit http://gerrit.cloudera.org:8080/20595 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id950e97782a2a29b505164470cfb646c5358dfca Gerrit-Change-Number: 20595 Gerrit-PatchSet: 2 Gerrit-Owner: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Comment-Date: Fri, 20 Oct 2023 09:28:58 +0000 Gerrit-HasComments: Yes