Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/21430 )
Change subject: IMPALA-12680: Fix NullPointerException during AlterTableAddPartitions ...................................................................... Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/21430/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21430/1//COMMIT_MSG@13 PS1, Line 13: processor > nit: processor Ack http://gerrit.cloudera.org:8080/#/c/21430/1/fe/src/main/java/org/apache/impala/service/BackendConfig.java File fe/src/main/java/org/apache/impala/service/BackendConfig.java: http://gerrit.cloudera.org:8080/#/c/21430/1/fe/src/main/java/org/apache/impala/service/BackendConfig.java@463 PS1, Line 463: : public String debugActions() { return backendCfg_.debug_actions; } : > Is this needed? Oops! I forgot to remove this as this is not being used. http://gerrit.cloudera.org:8080/#/c/21430/1/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/21430/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@5303 PS1, Line 5303: Map<String, Long> partitionToEventId, boolean > Can the @Nullable annotation removed here? Ack http://gerrit.cloudera.org:8080/#/c/21430/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@5336 PS1, Line 5336: addedHmsPartitions.addAll(partitionToEventSubMa > I think moving this Precondition to the beginning of function is sufficient Since we are not initializing 'partitionToEventId' to null anymore, I don't think we need this precondition any more. Regarding the test, I just wanted to simulate the real scenario mentioned in the Jira, so I would like to keep the debug actions. http://gerrit.cloudera.org:8080/#/c/21430/1/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java File fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java: http://gerrit.cloudera.org:8080/#/c/21430/1/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@4043 PS1, Line 4043: metaDataFilter.setNotificationFilter(null); > Is this variable necessary? BackendConfig.INSTANCE.setDebugActions() never Ack -- To view, visit http://gerrit.cloudera.org:8080/21430 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I730fed311ebc09762dccc152d9583d5394b0b9b3 Gerrit-Change-Number: 21430 Gerrit-PatchSet: 3 Gerrit-Owner: Sai Hemanth Gantasala <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[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, 28 May 2024 22:26:26 +0000 Gerrit-HasComments: Yes
