Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21175 )

Change subject: IMPALA-12829: Skip processing transaction events if the table 
is HMS sync disabled.
......................................................................


Patch Set 7:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/21175/7//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21175/7//COMMIT_MSG@17
PS7, Line 17: we need
            : special logic in the process() method.
Please explain the solution a little bit here.


http://gerrit.cloudera.org:8080/#/c/21175/7/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/21175/7/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@4516
PS7, Line 4516:     String tblFlagVal = tbl.getParameters()
              :         
.get(MetastoreEventPropertyKey.DISABLE_EVENT_HMS_SYNC.getKey());
              :     if (tblFlagVal != null ) {
              :       return Boolean.valueOf(tblFlagVal.trim());
              :     }
> getHmsSyncProperty() is a method of MetastoreTableEvent class and CommitTXN
I think we can use the static method directly, i.e. 
MetastoreTableEvent.getHmsSyncProperty(tbl). Or lift that method up to 
MetastoreEvent class.

This method can also be static and be moved into MetastoreEvent class.


http://gerrit.cloudera.org:8080/#/c/21175/7/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@4523
PS7, Line 4523:     if (Boolean.valueOf(dbFlagVal)) {
              :       return true;
              :     }
              :     return false;
nit: can be simplified to

 return Boolean.parseBoolean(dbFlagVal);


http://gerrit.cloudera.org:8080/#/c/21175/7/tests/custom_cluster/test_events_custom_configs.py
File tests/custom_cluster/test_events_custom_configs.py:

http://gerrit.cloudera.org:8080/#/c/21175/7/tests/custom_cluster/test_events_custom_configs.py@1506
PS7, Line 1506:     from tests.util.acid_txn import AcidTxn
This duplicates L34.


http://gerrit.cloudera.org:8080/#/c/21175/7/tests/custom_cluster/test_events_custom_configs.py@1512
PS7, Line 1512:       "CREATE TABLE {}.{} (i int) partitioned by (year int) {}"
Can we also test a non-partitioned table?


http://gerrit.cloudera.org:8080/#/c/21175/7/tests/custom_cluster/test_events_custom_configs.py@1515
PS7, Line 1515:     # self.client.execute("select * from {}.{}".format(txn_db, 
txn_table))
stale comment


http://gerrit.cloudera.org:8080/#/c/21175/7/tests/custom_cluster/test_events_custom_configs.py@1539
PS7, Line 1539:     assert partitions_refreshed_after == 
partitions_refreshed_before
These pass even without the fix. I think the reason is the table has no 
partitions in Impala's catalog cache since the ADD_PARTITION events generated 
by the insert statement are already skipped. We need to add the partition in 
Impala before inserting it in Hive.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5d0ecb3b756755bc04c66a538a9ae6b88011a019
Gerrit-Change-Number: 21175
Gerrit-PatchSet: 7
Gerrit-Owner: Sai Hemanth Gantasala <saihema...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <k.venureddy2...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <saihema...@cloudera.com>
Gerrit-Comment-Date: Tue, 29 Apr 2025 08:47:51 +0000
Gerrit-HasComments: Yes

Reply via email to