Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19410 )
Change subject: IMPALA-11834: Fix Iceberg LOAD DATA hdfsDelete JVM crash ...................................................................... Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/19410/1/be/src/service/client-request-state.h File be/src/service/client-request-state.h: http://gerrit.cloudera.org:8080/#/c/19410/1/be/src/service/client-request-state.h@27 PS1, Line 27: #include "runtime/hdfs-fs-cache.h" Do we use this include in this header? If not, can we move it to the .cc file? http://gerrit.cloudera.org:8080/#/c/19410/1/be/src/service/client-request-state.cc File be/src/service/client-request-state.cc: http://gerrit.cloudera.org:8080/#/c/19410/1/be/src/service/client-request-state.cc@878 PS1, Line 878: hdfs_ret Shouldn't we include the original error in hdfs_ret if we came here because of the condition "!hdfs_ret.ok()"? http://gerrit.cloudera.org:8080/#/c/19410/1/be/src/service/client-request-state.cc@886 PS1, Line 886: hdfs_ret Do we do anything with this status? we overwrite it on L889. http://gerrit.cloudera.org:8080/#/c/19410/1/be/src/service/client-request-state.cc@889 PS1, Line 889: Status Doesn't GetConnection() already return a Status, do we have to wrap it? http://gerrit.cloudera.org:8080/#/c/19410/1/be/src/service/client-request-state.cc@892 PS1, Line 892: hdfs_ret See L878. -- To view, visit http://gerrit.cloudera.org:8080/19410 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifb8f6ebf5b7100e69c1b02137d03fe70c331c30f Gerrit-Change-Number: 19410 Gerrit-PatchSet: 1 Gerrit-Owner: Tamas Mate <tma...@apache.org> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Gergely Fürnstáhl <gfurnst...@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-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Wed, 11 Jan 2023 12:38:19 +0000 Gerrit-HasComments: Yes
