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 4:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/21430/3/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/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1731
PS3, Line 1731: @Nullable Map<String, Long> partitionToEventId
> Can @Nullable annotation here dropped as well?
We cannot change it because it is being referenced in L#1721. Also the same 
applies to the HdfsTable class since there are null references to 
'partitionToEventId' from various callers.


http://gerrit.cloudera.org:8080/#/c/21430/3/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/3/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@4071
PS3, Line 4071: TPartitionDef partitionDef = new TPartitionDef();
              :       partitionDef.addToPartition_sp
> Since "enable_event_processor" restart EP, it will be great if we can also
Ack


http://gerrit.cloudera.org:8080/#/c/21430/3/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@4076
PS3, Line 4076: tsProcessor_.processEvents();
> Should this be a test assertion instead?
The finally block is just for precaution to not stop the event processor 
entirely because other tests may fail if this works unexpectedly. And then it 
would be hard to find the root cause if hundreds of unit tests fail.

I'll assert on the event processor status in the try block to be on the safe 
side.



--
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: 4
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: Thu, 30 May 2024 18:38:33 +0000
Gerrit-HasComments: Yes

Reply via email to