Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21437 )
Change subject: IMPALA-12277: Fix NullPointerException for partitioned inserts when EP is turned off ...................................................................... Patch Set 12: (4 comments) http://gerrit.cloudera.org:8080/#/c/21437/12/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/12/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7205 PS12, Line 7205: public TUpdateCatalogResponse updateCatalogImpl(TUpdateCatalogRequest update, This method is super long now. It'd be nice to refactor it by extracting some codes into methods. http://gerrit.cloudera.org:8080/#/c/21437/12/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7262 PS12, Line 7262: FeCatalogUtils.loadAllPartitions((FeFsTable)table); Instead of using the partition list in the cache, I think a cleaner approach is always fetching the updated partition list from HMS (without loading the file metadata). So we can also take care of new partitions added externally. The reason is that we will always fetch the updated partition list later in loadTableMetadata(). We can do it here to get rid of stale metadata cache. http://gerrit.cloudera.org:8080/#/c/21437/12/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7497 PS12, Line 7497: return updateCatalogImpl(update, false); // we should only retry once. It's not clear to me whether all the stuffs in updateCatalogImpl() are retryable, i.e. idempotent. E.g. do we fire duplicate HMS events? http://gerrit.cloudera.org:8080/#/c/21437/12/tests/custom_cluster/test_events_custom_configs.py File tests/custom_cluster/test_events_custom_configs.py: http://gerrit.cloudera.org:8080/#/c/21437/12/tests/custom_cluster/test_events_custom_configs.py@1279 PS12, Line 1279: self.client.execute("insert into {}.{} partition(year=2024) values (0)" Can we add more partitions to cover existing partitions? E.g. The table can originally have several partitions. One is dropped externally. Another new one (with a new name) is added externally. Then an INSERT in Impala inserts to all these partitions. -- 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: 12 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: Tue, 11 Jun 2024 03:04:21 +0000 Gerrit-HasComments: Yes
