Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/22554 )

Change subject: IMPALA-13593: Enable event processor to consume 
ALTER_PARTITIONS events from metastore
......................................................................


Patch Set 15:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/22554/11//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/22554/11//COMMIT_MSG@19
PS11, Line 19: . This can cause partitions to be refreshed even on
             : trivial changes to them.
> Agree. But HIVE-27746 is controlled with a config, and it is not turned on
Thanks for the explanation?
Can you create a ticket for this issue with HIVE-27746 and mention it here?


http://gerrit.cloudera.org:8080/#/c/22554/15/fe/src/compat-apache-hive-3/java/org/apache/impala/compat/MetastoreShim.java
File 
fe/src/compat-apache-hive-3/java/org/apache/impala/compat/MetastoreShim.java:

http://gerrit.cloudera.org:8080/#/c/22554/15/fe/src/compat-apache-hive-3/java/org/apache/impala/compat/MetastoreShim.java@476
PS15, Line 476: getPartitions
getPartitionsAfter()?


http://gerrit.cloudera.org:8080/#/c/22554/15/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java:

http://gerrit.cloudera.org:8080/#/c/22554/15/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@611
PS15, Line 611: returns them in a map.
this should be updated


http://gerrit.cloudera.org:8080/#/c/22554/15/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@1132
PS15, Line 1132: TableReloadInfo
I don't get the naming - this class seems specific to ALTER_PARTITIONS, but 
this is not reflected in the name. Or this could be used in other types too in 
the future?


http://gerrit.cloudera.org:8080/#/c/22554/15/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@1150
PS15, Line 1150: getMsTable
Maybe getMsTableAfter would be more exact?


http://gerrit.cloudera.org:8080/#/c/22554/15/tests/custom_cluster/test_events_custom_configs.py
File tests/custom_cluster/test_events_custom_configs.py:

http://gerrit.cloudera.org:8080/#/c/22554/15/tests/custom_cluster/test_events_custom_configs.py@1677
PS15, Line 1677:       assert batch_events_after == batch_events_before  # 
verify there are no new batches
Can we also verify to the total number of events generated by ANALYZE TABLE?
Also, should this case generate an ALTER_PARTITIONS event? We could verify it 
similarly to other cases.


http://gerrit.cloudera.org:8080/#/c/22554/15/tests/custom_cluster/test_events_custom_configs.py@1704
PS15, Line 1704:       event_found = False
               :       for event in events:
               :         if event.eventType == "ALTER_PARTITIONS":
               :           event_found = True
               :         else:
               :           logging.debug("Found event: " + str(event))
               :       assert event_found
The same is done twice, there could be a function for it



--
To view, visit http://gerrit.cloudera.org:8080/22554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I009a87ef5e2c331272f9e2d7a6342cc860e64737
Gerrit-Change-Number: 22554
Gerrit-PatchSet: 15
Gerrit-Owner: Sai Hemanth Gantasala <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Daniel Becker <[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: Fri, 08 Aug 2025 16:23:31 +0000
Gerrit-HasComments: Yes

Reply via email to