Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/21437 )
Change subject: IMPALA-12277: Fix NullPointerException for partitioned inserts when partition list is stale ...................................................................... Patch Set 15: (10 comments) http://gerrit.cloudera.org:8080/#/c/21437/14//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21437/14//COMMIT_MSG@7 PS14, Line 7: when : partition list i > nit: let's make this accurate, e.g. "when partition list is stale" Ack http://gerrit.cloudera.org:8080/#/c/21437/14//COMMIT_MSG@18 PS14, Line 18: always verify the partitions from HMS witho > nit: "always verify the partitions" ? Ack http://gerrit.cloudera.org:8080/#/c/21437/13/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: http://gerrit.cloudera.org:8080/#/c/21437/13/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1292 PS13, Line 1292: nullPartitionKeyValue_ = > nit: as we touch this code, can we add an error message for Preconditions.c Ack http://gerrit.cloudera.org:8080/#/c/21437/14/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: http://gerrit.cloudera.org:8080/#/c/21437/14/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1238 PS14, Line 1238: Action, partitionToEventId, re > nit: we usually put the comment before the value, e.g. L1225-L1227 Ack http://gerrit.cloudera.org:8080/#/c/21437/14/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1265 PS14, Line 1265: * Existing file descriptors might be reused incorrectly if Hdfs rebalancer was : * executed, as it changes the block locations but doesn't update the mtime (file : * modification time). : * If this occurs, user has to execute "invalidate metadata" to invalidate the : * metadata cache of the table and trigger a fresh load. : */ > This method has 12 parameter now. That is quite long. I have addressed this comment to the newly introduced method. Created https://issues.apache.org/jira/browse/IMPALA-13171 to track and address the older load() methods. http://gerrit.cloudera.org:8080/#/c/21437/14/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1270 PS14, Line 1270: > Add documentation for isPreLoadForInsert parameter. Ack http://gerrit.cloudera.org:8080/#/c/21437/13/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/21437/13/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7253 PS13, Line 7253: e.get > nit: could you add a comment to highlight this is 'loadPartitionFileMetadat Ack http://gerrit.cloudera.org:8080/#/c/21437/13/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7279 PS13, Line 7279: Set of all partition names targeted > nit: let's avoid using the variable name 'partitionsToCreate' in the method Ack http://gerrit.cloudera.org:8080/#/c/21437/13/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7405 PS13, Line 7405: > nit: remove one space here Ack http://gerrit.cloudera.org:8080/#/c/21437/13/tests/custom_cluster/test_events_custom_configs.py File tests/custom_cluster/test_events_custom_configs.py: http://gerrit.cloudera.org:8080/#/c/21437/13/tests/custom_cluster/test_events_custom_configs.py@1275 PS13, Line 1275: EventProcessorUtils.wait_for_event_processing(self) > Can we also add year=2022 as an existing partition? Now we have stale parti Ack -- To view, visit http://gerrit.cloudera.org:8080/21437 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ide8f1f6bf017e9a040b53bb5d5291ff2ea3e0d18 Gerrit-Change-Number: 21437 Gerrit-PatchSet: 15 Gerrit-Owner: Sai Hemanth Gantasala <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]> Gerrit-Comment-Date: Fri, 21 Jun 2024 00:54:36 +0000 Gerrit-HasComments: Yes
