Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/21019 )
Change subject: IMPALA-12487: Skip reloading file metadata for ALTER_TABLE events with trivial changes in StorageDescriptor ...................................................................... Patch Set 9: (6 comments) http://gerrit.cloudera.org:8080/#/c/21019/8/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/21019/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1899 PS8, Line 1899: (isTrivialSdPropsChang > Can't this return true when the SD didn't change at all but there are other Good point! This is what I'm missing. We should check for SD changes only if we notice any changes in SD. http://gerrit.cloudera.org:8080/#/c/21019/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1963 PS8, Line 1963: > Is it possible that on is null while the other is not? Shouldn't the functi I have addressed this at L#1899 http://gerrit.cloudera.org:8080/#/c/21019/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1965 PS8, Line 1965: // Check if the trivial SD properties are changed during the alter statement > nit: this looks a bit unusual in Impala, can you split it to to two lines? Ack http://gerrit.cloudera.org:8080/#/c/21019/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1967 PS8, Line 1967: StorageDescriptor afterSd) { > non-trivial could be added to the log message Done http://gerrit.cloudera.org:8080/#/c/21019/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1977 PS8, Line 1977: > I think that it would make the intention of the function clearer to just st Ack http://gerrit.cloudera.org:8080/#/c/21019/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1987 PS8, Line 1987: sd.unsetCols(); > Can you add a comment for this? The previous lines just unset the propertie Done -- To view, visit http://gerrit.cloudera.org:8080/21019 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6fd9a9504bf93d2529dc7accbf436ad83e51d8ac Gerrit-Change-Number: 21019 Gerrit-PatchSet: 9 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: Thu, 21 Mar 2024 00:12:29 +0000 Gerrit-HasComments: Yes
