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

Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE 
events
......................................................................


Patch Set 6:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc
File be/src/catalog/catalog-server.cc:

http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc@136
PS2, Line 136: "configuration is used to whitelist the table properti
> About change of 'transactional': there is a test where it changes, though '
Ok then, so we don't need to include 'transactional' in the whitelist config.
For, transactional_properties, changing between insert_only and default would 
change the table metadata. File metadata will not be changed since the table is 
being translated from managed to external, so file metadata will not be 
modified.


http://gerrit.cloudera.org:8080/#/c/19838/4/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/19838/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@374
PS4, Line 374: String whitelist = Backen
> Iceberg also use table property 'metadata_location' to denote the current s
Ack. Good point!!


http://gerrit.cloudera.org:8080/#/c/19838/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@377
PS4, Line 377:
> My impression is that table property names are also case sensitive: I was a
Hmm so, do you think we should add 'EXTERNAL' also to the whitelist config?


http://gerrit.cloudera.org:8080/#/c/19838/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3936
PS4, Line 3936:     for (String whitelistConfig : whitelistedTblProperties_) {
              :       String configBefore = 
beforeTable.getParameters().get(whitelistConfig);
              :       String configAfter = 
afterTable.getParameters().get(whitelistConfig);
              :       if (!Objects.equals(configBefore, configAfter)) return 
true;
              :     }
              :     return false;
              :   }
              : }
              :
> Can it be replaced with:
Ack



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 6
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-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Wed, 31 May 2023 16:37:44 +0000
Gerrit-HasComments: Yes

Reply via email to