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

Change subject: IMPALA-10976: Sync db/table to latest HMS event for all DDL/DMLs
......................................................................


Patch Set 34:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/20367/33/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java:

http://gerrit.cloudera.org:8080/#/c/20367/33/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@1401
PS33, Line 1401: trace
> Let's use trace() since this will be logged for each new loaded partition.
Ack


http://gerrit.cloudera.org:8080/#/c/20367/33/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/20367/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3357
PS33, Line 3357:
               :       }
               :       if 
(!isTableBeingReplicated(metaStoreClient.getHiveClient(), hdfsTable) &&
               :           !syncToLatestEventId) {
               :         // when table is replicated we let the HMS API handle 
the file deletion logic
               :         // otherwise we delete the files.
               :         Collection<? extends FeFsPartition> parts = 
FeCatalogUtils
               :             .loadAllPartitions(hdfsTable);
               :         for (FeFsPartition part : parts) {
               :           FileSystemUtil.deleteAllVisibleFiles(new 
Path(part.getLocation()));
               :         }
> It'd be better to keep these unchanged since previously they are outside th
Ack


http://gerrit.cloudera.org:8080/#/c/20367/33/tests/common/impala_test_suite.py
File tests/common/impala_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/20367/33/tests/common/impala_test_suite.py@958
PS33, Line 958:     result = cls.__execute_query(impalad_client, query, 
query_options, user)
> Let's check success first, i.e. like what we does in execute_query_expect_s
Ack


http://gerrit.cloudera.org:8080/#/c/20367/33/tests/common/impala_test_suite.py@1046
PS33, Line 1046:
> nit: use 'cls' for classmethod
Ack


http://gerrit.cloudera.org:8080/#/c/20367/33/tests/common/impala_test_suite.py@1052
PS33, Line 1052:
> nit: use 'cls' for classmethod
Ack



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia250d0a943838086c187e5cb7c60035e5a564bbf
Gerrit-Change-Number: 20367
Gerrit-PatchSet: 34
Gerrit-Owner: Sai Hemanth Gantasala <[email protected]>
Gerrit-Reviewer: Anonymous Coward <[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, 29 Jan 2024 19:02:50 +0000
Gerrit-HasComments: Yes

Reply via email to