Noemi Pap-Takacs has posted comments on this change. ( http://gerrit.cloudera.org:8080/20903 )
Change subject: IMPALA-12708: An UPDATE creates 2 new snapshots in Iceberg tables ...................................................................... Patch Set 1: (1 comment) Thanks for the fix and for refactoring the code. LGTM http://gerrit.cloudera.org:8080/#/c/20903/1/be/src/service/client-request-state.cc File be/src/service/client-request-state.cc: http://gerrit.cloudera.org:8080/#/c/20903/1/be/src/service/client-request-state.cc@1610 PS1, Line 1610: noop = false 'noop' is short and descriptive, but positive phrasing would make the code easier to understand. Do you think it helps if we define a flag that holds the same value as the return value of the function? E.g. 'update_catalog', which could be true by default and false if no catalog update is needed. -- To view, visit http://gerrit.cloudera.org:8080/20903 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2ceb80b939c644388707b21061bf55451234dcd3 Gerrit-Change-Number: 20903 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Noemi Pap-Takacs <npaptak...@cloudera.com> Gerrit-Reviewer: Tamas Mate <tma...@apache.org> Gerrit-Comment-Date: Wed, 17 Jan 2024 12:48:01 +0000 Gerrit-HasComments: Yes
