Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/20204 )
Change subject: IMPALA-12072: Include snapshot id of Iceberg tables in query plan / profile ...................................................................... Patch Set 11: Code-Review+1 (2 comments) Thanks for the clarifications Zoltan. LGTM! http://gerrit.cloudera.org:8080/#/c/20204/9/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java File fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java: http://gerrit.cloudera.org:8080/#/c/20204/9/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@132 PS9, Line 132: 0 nit: in Impala I have found -1s as invalid snapshot ids. http://gerrit.cloudera.org:8080/#/c/20204/9/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@145 PS9, Line 145: IcebergUtil.getSnapshotId(getIceTable(), tblRef_.getTimeTravelSpec()); > An IcebergScanPlanner can create up to 3 IcebergScanNode objects, so we can Thanks for the explanation Zoltan, it totally makes sense to keep it as is. My intent with the comment was to keep the constructor light if possible. In case someone in the future has to instantiate an IcebergScanNode in a place where it is difficult to obtain. -- To view, visit http://gerrit.cloudera.org:8080/20204 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee0b4967429ea733729ad8e44df32e3b24b88525 Gerrit-Change-Number: 20204 Gerrit-PatchSet: 11 Gerrit-Owner: Gergely Fürnstáhl <g.furnst...@gmail.com> Gerrit-Reviewer: Andrew Sherman <asher...@cloudera.com> Gerrit-Reviewer: Gergely Fürnstáhl <g.furnst...@gmail.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Tamas Mate <tma...@apache.org> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Thu, 01 Feb 2024 18:41:58 +0000 Gerrit-HasComments: Yes
