Sai Hemanth Gantasala has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23487 )

Change subject: IMPALA-14131: Add flag to configure the default value of 
'impala.disableHmsSync'
......................................................................


Patch Set 5:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/23487/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:

http://gerrit.cloudera.org:8080/#/c/23487/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@4832
PS3, Line 4832:
> To be clear, let's add the flag name.
Ack


http://gerrit.cloudera.org:8080/#/c/23487/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java:

http://gerrit.cloudera.org:8080/#/c/23487/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1265
PS3, Line 1265:             msTbl_.getTableName(), dbName_, 
globalDisableHmsSync);
> Let's mention the tabel/db level key name and the flag name here as well.
Ack


http://gerrit.cloudera.org:8080/#/c/23487/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java:

http://gerrit.cloudera.org:8080/#/c/23487/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1239
PS4, Line 1239:       Preconditions.checkNotNull(msTbl_);
> nit: this comment is unrelated to the following code. Pls move this to L126
Apologies, this comment is for me while writing the code.


http://gerrit.cloudera.org:8080/#/c/23487/4/tests/custom_cluster/test_events_custom_configs.py
File tests/custom_cluster/test_events_custom_configs.py:

http://gerrit.cloudera.org:8080/#/c/23487/4/tests/custom_cluster/test_events_custom_configs.py@1861
PS4, Line 1861:     self.client.execute(
              :       """alter table {} SET TBLPROPERTIES 
('impala.disableHmsSync'='false')"""
              :       .format(tbl1))
> I ran the test and found that processing ALTER_TABLE event from this invali
Ack



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4ee617aed48575502d9cf5cf2cbea6ec897d6839
Gerrit-Change-Number: 23487
Gerrit-PatchSet: 5
Gerrit-Owner: Sai Hemanth Gantasala <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]>
Gerrit-Comment-Date: Mon, 17 Nov 2025 23:37:01 +0000
Gerrit-HasComments: Yes

Reply via email to