Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/19547 )
Change subject: IMPALA-11950: Planner change for Iceberg metadata querying ...................................................................... Patch Set 4: (5 comments) Thank you for the review Daniel, updated the change. http://gerrit.cloudera.org:8080/#/c/19547/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19547/4//COMMIT_MSG@7 PS4, Line 7: IMPALA-11950: Planner change for Iceberg metadata querying > I wonder if we should add something about the changes in processing cost ca That is a different change that appeared due to the rebase. http://gerrit.cloudera.org:8080/#/c/19547/4/fe/src/main/java/org/apache/impala/planner/ScanNode.java File fe/src/main/java/org/apache/impala/planner/ScanNode.java: http://gerrit.cloudera.org:8080/#/c/19547/4/fe/src/main/java/org/apache/impala/planner/ScanNode.java@368 PS4, Line 368: else > Nit: no need for else because the THEN branch returns. This isn't my change, I only removed some unused code I noticed while investigating but it looks like that code was removed as well in a different change. I will keep this file as is for now. http://gerrit.cloudera.org:8080/#/c/19547/4/fe/src/main/java/org/apache/impala/planner/ScanNode.java@390 PS4, Line 390: reflect > Nit: reflects. This line appeared due to the rebase. I will keep this file as is for now http://gerrit.cloudera.org:8080/#/c/19547/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java: http://gerrit.cloudera.org:8080/#/c/19547/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@866 PS4, Line 866: enclosingTupleDescs > This variable is not used. This part was added here because of a rebase, ri Yeah, chaining the changes was not the best idea, I was hoping for better support from gerrit. Anyway, removed the unused variable, it makes the codebase cleaner. http://gerrit.cloudera.org:8080/#/c/19547/4/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/19547/4/fe/src/main/java/org/apache/impala/service/Frontend.java@294 PS4, Line 294: that > Remove 'that'? Done -- To view, visit http://gerrit.cloudera.org:8080/19547 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3675d7a57ca570bfec306798589b5ef6aa34b5c6 Gerrit-Change-Number: 19547 Gerrit-PatchSet: 4 Gerrit-Owner: Tamas Mate <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Gergely Fürnstáhl <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tamas Mate <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Mon, 17 Apr 2023 08:52:21 +0000 Gerrit-HasComments: Yes
