Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21430 )
Change subject: IMPALA-12680: Fix NullPointerException during AlterTableAddPartitions ...................................................................... Patch Set 1: (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: processer nit: processor 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 void setDebugActions(String debugActions) { : backendCfg_.debug_actions = debugActions; : } Is this needed? AFAIK, all debugAction in CatalogOpExecutor comes from the thrift message(either TDdlExecRequest or TUpdateCatalogRequest). 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: @Nullable Map<String, Long> partitionToEventId Can the @Nullable annotation removed here? http://gerrit.cloudera.org:8080/#/c/21430/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@5336 PS1, Line 5336: Preconditions.checkNotNull(partitionToEventId); I think moving this Precondition to the beginning of function is sufficient to test this change. You can stop EP entirely and run testAlterTableWithEpDisabled without specifying new debug action to restart EP. The assertion is that partitionToEventId must never be null. 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: String prevDebugActions = BackendConfig.INSTANCE.debugActions(); Is this variable necessary? BackendConfig.INSTANCE.setDebugActions() never called with other value. -- 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: 1 Gerrit-Owner: Sai Hemanth Gantasala <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Fri, 24 May 2024 00:17:50 +0000 Gerrit-HasComments: Yes
