This is an automated email from the ASF dual-hosted git repository. laszlog pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit 1f7b9601e5a768c0b2061fef95c750ae74059b84 Author: Sai Hemanth Gantasala <[email protected]> AuthorDate: Thu Oct 3 15:53:36 2024 -0700 IMPALA-13403: Refactor the checks of skip reloading file metadata for ALTER_TABLE events IMPALA-12487 adds an optimization that if an ALTER_TABLE event has trivial changes in StorageDescriptor (e.g. removing optional field 'storedAsSubDirectories'=false which defaults to false), file metadata reload will be skipped, no matter what changes are in the table properties. This is problematic since some HMS clients (e.g. Spark) could modify both the table properties and StorageDescriptor. If there is a non-trivial changes in table properties (e.g. 'location' change), we shouldn't skip reloading file metadata. Testing: - Added a unit test to verify the same Change-Id: Ia969dd32385ac5a1a9a65890a5ccc8cd257f4b97 Reviewed-on: http://gerrit.cloudera.org:8080/21971 Reviewed-by: Impala Public Jenkins <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> --- .../impala/catalog/events/MetastoreEvents.java | 20 +++++++++----------- .../catalog/events/MetastoreEventsProcessorTest.java | 10 ++++++++++ 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java b/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java index 43ec31549..2b1d21a7e 100644 --- a/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java +++ b/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java @@ -1945,19 +1945,17 @@ public class MetastoreEvents { // There are lot of other alter statements which doesn't require file metadata // reload but these are the most common types for alter statements. - boolean skipFileMetadata = false; - if (isFieldSchemaChanged(beforeTable, afterTable) || - isTableOwnerChanged(beforeTable.getOwner(), afterTable.getOwner())) { - skipFileMetadata = true; - } else if (!Objects.equals(beforeTable.getSd(), afterTable.getSd())) { - if (isTrivialSdPropsChanged(beforeTable.getSd(), afterTable.getSd())) { - skipFileMetadata = true; + if (!Objects.equals(beforeTable.getSd(), afterTable.getSd())) { + if (!isTrivialSdPropsChanged(beforeTable.getSd(), afterTable.getSd())) { + return false; } - } else if (!isCustomTblPropsChanged(whitelistedTblProperties, beforeTable, - afterTable)) { - skipFileMetadata = true; } - return skipFileMetadata; + if (isCustomTblPropsChanged(whitelistedTblProperties, beforeTable, afterTable)) { + return false; + } + infoLog("Skipping reloading of file metadata for table {}.{} since SD and " + + "whitelistedTblProperties has not changed.", dbName_, tblName_); + return true; } private boolean isFieldSchemaChanged( diff --git a/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java b/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java index a781ac985..e550bfdb1 100644 --- a/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java +++ b/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java @@ -3246,6 +3246,16 @@ public class MetastoreEventsProcessorTest { hmsTbl.getParameters().put("EXTERNAL", "false"); fileMetadataLoadAfter = processAlterTableAndReturnMetric(testTblName, hmsTbl); assertEquals(fileMetadataLoadBefore + 1, fileMetadataLoadAfter); + + // Test 9: Change trivial SD props and also change non-trivial + // TblProperties to verify file metadata is reloaded + LOG.info("Test changes in non-trivial tbl props and trivial SD"); + hmsTbl = tbl.getMetaStoreTable().deepCopy(); + hmsTbl.getSd().setBucketCols(Arrays.asList("c1")); + hmsTbl.getParameters().put("EXTERNAL", "true"); + fileMetadataLoadBefore = fileMetadataLoadAfter; + fileMetadataLoadAfter = processAlterTableAndReturnMetric(testTblName, hmsTbl); + assertEquals(fileMetadataLoadBefore + 1, fileMetadataLoadAfter); } private long processAlterTableAndReturnMetric(String testTblName,
