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 13: (6 comments) http://gerrit.cloudera.org:8080/#/c/19838/11/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/19838/11/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1469 PS11, Line 1469: if (canBeSkipped()) { > This also checks tblproperties but acts like blacklist (i.e. skip if match) This skips the processing alter table event itself. In this propose to skipFilemetadata reload only. So I think we should keep this. http://gerrit.cloudera.org:8080/#/c/19838/11/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1537 PS11, Line 1537: infoLog("Change in number of columns detected for table {}.{} from {} to {}", > nit: let's try to log the numbers. Also use fully qualified table name to e Ack http://gerrit.cloudera.org:8080/#/c/19838/11/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1546 PS11, Line 1546: infoLog("Change in table schema detected for table {}.{} from {} to {} ", > nit: let's try to log the new and old name and type of this column Ack http://gerrit.cloudera.org:8080/#/c/19838/11/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1555 PS11, Line 1555: > nit: let's try to log the new and old owners, e.g. Ack http://gerrit.cloudera.org:8080/#/c/19838/11/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1568 PS11, Line 1568: org.apache.hadoop.hive.metastore.api.Table afterTable) { > nit: let's log the new and old strings Ack http://gerrit.cloudera.org:8080/#/c/19838/11/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java File fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java: http://gerrit.cloudera.org:8080/#/c/19838/11/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@2813 PS11, Line 2813: alterTableSetOwner(testTblNa > Shouldn't we do this in Hive? Currently, there is no method to change the owner from HMS. I have now added a method to change it from HMS. -- 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: 13 Gerrit-Owner: Sai Hemanth Gantasala <saihema...@cloudera.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-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Wed, 14 Jun 2023 21:07:13 +0000 Gerrit-HasComments: Yes
