From c39100e1bd1b400f69b7c3ef516e1b1fa68a3e42 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 22 Apr 2025 10:38:03 -0700 Subject: [PATCH 01/12] initial copy paste --- .../catalog/iceberg/IcebergCatalog.java | 260 ++++++++++++++++-- 1 file changed, 232 insertions(+), 28 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index f80d4077ab..5f4140dec3 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -35,10 +35,14 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.UUID; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -46,11 +50,14 @@ import org.apache.iceberg.BaseMetastoreTableOperations; import org.apache.iceberg.BaseTable; import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.CatalogUtil; +import org.apache.iceberg.LocationProviders; import org.apache.iceberg.Schema; import org.apache.iceberg.Table; import org.apache.iceberg.TableMetadata; import org.apache.iceberg.TableMetadataParser; import org.apache.iceberg.TableOperations; +import org.apache.iceberg.TableProperties; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.SupportsNamespaces; import org.apache.iceberg.catalog.TableIdentifier; @@ -68,7 +75,11 @@ import org.apache.iceberg.io.CloseableGroup; import org.apache.iceberg.io.FileIO; import org.apache.iceberg.io.InputFile; +import org.apache.iceberg.io.LocationProvider; +import org.apache.iceberg.io.OutputFile; +import org.apache.iceberg.util.LocationUtil; import org.apache.iceberg.util.PropertyUtil; +import org.apache.iceberg.util.Tasks; import org.apache.iceberg.view.BaseMetastoreViewCatalog; import org.apache.iceberg.view.BaseViewOperations; import org.apache.iceberg.view.ViewBuilder; @@ -1190,11 +1201,18 @@ public ViewBuilder withLocation(String newLocation) { } } - private class BasePolarisTableOperations extends BaseMetastoreTableOperations { + private class BasePolarisTableOperations implements TableOperations { private final TableIdentifier tableIdentifier; private final String fullTableName; private FileIO tableFileIO; + private static final String METADATA_FOLDER_NAME = "metadata"; + + private TableMetadata currentMetadata = null; + private String currentMetadataLocation = null; + private boolean shouldRefresh = true; + private int version = -1; + BasePolarisTableOperations(FileIO defaultFileIO, TableIdentifier tableIdentifier) { LOGGER.debug("new BasePolarisTableOperations for {}", tableIdentifier); this.tableIdentifier = tableIdentifier; @@ -1202,7 +1220,6 @@ private class BasePolarisTableOperations extends BaseMetastoreTableOperations { this.tableFileIO = defaultFileIO; } - @Override public void doRefresh() { LOGGER.debug("doRefresh for tableIdentifier {}", tableIdentifier); // While doing refresh/commit protocols, we must fetch the fresh "passthrough" resolved @@ -1250,7 +1267,40 @@ public void doRefresh() { } } - @Override + protected void refreshFromMetadataLocation( + String newLocation, + Predicate shouldRetry, + int numRetries, + Function metadataLoader) { + // use null-safe equality check because new tables have a null metadata location + if (!Objects.equal(currentMetadataLocation, newLocation)) { + LOGGER.info("Refreshing table metadata from new version: {}", newLocation); + + AtomicReference newMetadata = new AtomicReference<>(); + Tasks.foreach(newLocation) + .retry(numRetries) + .exponentialBackoff(100, 5000, 600000, 4.0 /* 100, 400, 1600, ... */) + .throwFailureWhenFinished() + .stopRetryOn(NotFoundException.class) // overridden if shouldRetry is non-null + .shouldRetryTest(shouldRetry) + .run(metadataLocation -> newMetadata.set(metadataLoader.apply(metadataLocation))); + + String newUUID = newMetadata.get().uuid(); + if (currentMetadata != null && currentMetadata.uuid() != null && newUUID != null) { + Preconditions.checkState( + newUUID.equals(currentMetadata.uuid()), + "Table UUID does not match: current=%s != refreshed=%s", + currentMetadata.uuid(), + newUUID); + } + + this.currentMetadata = newMetadata.get(); + this.currentMetadataLocation = newLocation; + this.version = parseVersion(newLocation); + } + this.shouldRefresh = false; + } + public void doCommit(TableMetadata base, TableMetadata metadata) { LOGGER.debug( "doCommit for table {} with base {}, metadata {}", tableIdentifier, base, metadata); @@ -1395,41 +1445,167 @@ public void doCommit(TableMetadata base, TableMetadata metadata) { } } + public int currentVersion() { + return version; + } + + protected void requestRefresh() { + this.shouldRefresh = true; + } + + protected void disableRefresh() { + this.shouldRefresh = false; + } + + protected String writeNewMetadataIfRequired(boolean newTable, TableMetadata metadata) { + return newTable && metadata.metadataFileLocation() != null + ? metadata.metadataFileLocation() + : writeNewMetadata(metadata, currentVersion() + 1); + } + + protected String writeNewMetadata(TableMetadata metadata, int newVersion) { + String newTableMetadataFilePath = newTableMetadataFilePath(metadata, newVersion); + OutputFile newMetadataLocation = io().newOutputFile(newTableMetadataFilePath); + + // write the new metadata + // use overwrite to avoid negative caching in S3. this is safe because the metadata location is + // always unique because it includes a UUID. + TableMetadataParser.overwrite(metadata, newMetadataLocation); + + return newMetadataLocation.location(); + } + + @Override + public TableMetadata current() { + if (shouldRefresh) { + return refresh(); + } + return currentMetadata; + } + + @Override + public TableMetadata refresh() { + boolean currentMetadataWasAvailable = currentMetadata != null; + try { + doRefresh(); + } catch (NoSuchTableException e) { + if (currentMetadataWasAvailable) { + LOGGER.warn("Could not find the table during refresh, setting current metadata to null", e); + shouldRefresh = true; + } + + currentMetadata = null; + currentMetadataLocation = null; + version = -1; + throw e; + } + return current(); + } + + @Override + public void commit(TableMetadata base, TableMetadata metadata) { + // if the metadata is already out of date, reject it + if (base != current()) { + if (base != null) { + throw new CommitFailedException("Cannot commit: stale table metadata"); + } else { + // when current is non-null, the table exists. but when base is null, the commit is trying + // to create the table + throw new AlreadyExistsException("Table already exists: %s", tableName()); + } + } + // if the metadata is not changed, return early + if (base == metadata) { + LOGGER.info("Nothing to commit."); + return; + } + + long start = System.currentTimeMillis(); + doCommit(base, metadata); + CatalogUtil.deleteRemovedMetadataFiles(io(), base, metadata); + requestRefresh(); + + LOGGER.info( + "Successfully committed to table {} in {} ms", + tableName(), + System.currentTimeMillis() - start); + } + @Override public FileIO io() { return tableFileIO; } @Override + public String metadataFileLocation(String filename) { + return metadataFileLocation(current(), filename); + } + + @Override + public LocationProvider locationProvider() { + return LocationProviders.locationsFor(current().location(), current().properties()); + } + protected String tableName() { return fullTableName; } - } - private void validateMetadataFileInTableDir( - TableIdentifier identifier, TableMetadata metadata, CatalogEntity catalog) { - PolarisCallContext polarisCallContext = callContext.getPolarisCallContext(); - boolean allowEscape = - polarisCallContext - .getConfigurationStore() - .getConfiguration( - polarisCallContext, FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION); - if (!allowEscape - && !polarisCallContext - .getConfigurationStore() - .getConfiguration( - polarisCallContext, FeatureConfiguration.ALLOW_EXTERNAL_METADATA_FILE_LOCATION)) { - LOGGER.debug( - "Validating base location {} for table {} in metadata file {}", - metadata.location(), - identifier, - metadata.metadataFileLocation()); - StorageLocation metadataFileLocation = StorageLocation.of(metadata.metadataFileLocation()); - StorageLocation baseLocation = StorageLocation.of(metadata.location()); - if (!metadataFileLocation.isChildOf(baseLocation)) { - throw new BadRequestException( - "Metadata location %s is not allowed outside of table location %s", - metadata.metadataFileLocation(), metadata.location()); + private String metadataFileLocation(TableMetadata metadata, String filename) { + String metadataLocation = metadata.properties().get(TableProperties.WRITE_METADATA_LOCATION); + + if (metadataLocation != null) { + return String.format("%s/%s", LocationUtil.stripTrailingSlash(metadataLocation), filename); + } else { + return String.format("%s/%s/%s", metadata.location(), METADATA_FOLDER_NAME, filename); + } + } + + /** + * Validate if the new metadata location is the current metadata location or present within + * previous metadata files. + * + * @param newMetadataLocation newly written metadata location + * @return true if the new metadata location is the current metadata location or present within + * previous metadata files. + */ + private boolean checkCurrentMetadataLocation(String newMetadataLocation) { + TableMetadata metadata = refresh(); + String currentMetadataFileLocation = metadata.metadataFileLocation(); + return currentMetadataFileLocation.equals(newMetadataLocation) + || metadata.previousFiles().stream() + .anyMatch(log -> log.file().equals(newMetadataLocation)); + } + + private String newTableMetadataFilePath(TableMetadata meta, int newVersion) { + String codecName = + meta.property( + TableProperties.METADATA_COMPRESSION, TableProperties.METADATA_COMPRESSION_DEFAULT); + String fileExtension = TableMetadataParser.getFileExtension(codecName); + return metadataFileLocation( + meta, + String.format(Locale.ROOT, "%05d-%s%s", newVersion, UUID.randomUUID(), fileExtension)); + } + + /** + * Parse the version from table metadata file name. + * + * @param metadataLocation table metadata file location + * @return version of the table metadata file in success case and -1 if the version is not + * parsable (as a sign that the metadata is not part of this catalog) + */ + private static int parseVersion(String metadataLocation) { + int versionStart = metadataLocation.lastIndexOf('/') + 1; // if '/' isn't found, this will be 0 + int versionEnd = metadataLocation.indexOf('-', versionStart); + if (versionEnd < 0) { + // found filesystem table's metadata + return -1; + } + + try { + return Integer.parseInt(metadataLocation.substring(versionStart, versionEnd)); + } catch (NumberFormatException e) { + LOGGER.warn("Unable to parse version from metadata location: {}", metadataLocation, e); + return -1; } } } @@ -1600,6 +1776,34 @@ protected String viewName() { } } + private void validateMetadataFileInTableDir( + TableIdentifier identifier, TableMetadata metadata, CatalogEntity catalog) { + PolarisCallContext polarisCallContext = callContext.getPolarisCallContext(); + boolean allowEscape = + polarisCallContext + .getConfigurationStore() + .getConfiguration( + polarisCallContext, FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION); + if (!allowEscape + && !polarisCallContext + .getConfigurationStore() + .getConfiguration( + polarisCallContext, FeatureConfiguration.ALLOW_EXTERNAL_METADATA_FILE_LOCATION)) { + LOGGER.debug( + "Validating base location {} for table {} in metadata file {}", + metadata.location(), + identifier, + metadata.metadataFileLocation()); + StorageLocation metadataFileLocation = StorageLocation.of(metadata.metadataFileLocation()); + StorageLocation baseLocation = StorageLocation.of(metadata.location()); + if (!metadataFileLocation.isChildOf(baseLocation)) { + throw new BadRequestException( + "Metadata location %s is not allowed outside of table location %s", + metadata.metadataFileLocation(), metadata.location()); + } + } + } + private FileIO loadFileIOForTableLike( TableIdentifier identifier, Set readLocations, From f3f931824c334a9cd4b4f2c449c3b9c6c11510b6 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 22 Apr 2025 10:49:13 -0700 Subject: [PATCH 02/12] Reorder --- .../catalog/iceberg/IcebergCatalog.java | 142 +++++++++--------- 1 file changed, 71 insertions(+), 71 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index 5f4140dec3..f346dbaf3c 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -1220,6 +1220,77 @@ private class BasePolarisTableOperations implements TableOperations { this.tableFileIO = defaultFileIO; } + @Override + public TableMetadata current() { + if (shouldRefresh) { + return refresh(); + } + return currentMetadata; + } + + @Override + public TableMetadata refresh() { + boolean currentMetadataWasAvailable = currentMetadata != null; + try { + doRefresh(); + } catch (NoSuchTableException e) { + if (currentMetadataWasAvailable) { + LOGGER.warn("Could not find the table during refresh, setting current metadata to null", e); + shouldRefresh = true; + } + + currentMetadata = null; + currentMetadataLocation = null; + version = -1; + throw e; + } + return current(); + } + + @Override + public void commit(TableMetadata base, TableMetadata metadata) { + // if the metadata is already out of date, reject it + if (base != current()) { + if (base != null) { + throw new CommitFailedException("Cannot commit: stale table metadata"); + } else { + // when current is non-null, the table exists. but when base is null, the commit is trying + // to create the table + throw new AlreadyExistsException("Table already exists: %s", tableName()); + } + } + // if the metadata is not changed, return early + if (base == metadata) { + LOGGER.info("Nothing to commit."); + return; + } + + long start = System.currentTimeMillis(); + doCommit(base, metadata); + CatalogUtil.deleteRemovedMetadataFiles(io(), base, metadata); + requestRefresh(); + + LOGGER.info( + "Successfully committed to table {} in {} ms", + tableName(), + System.currentTimeMillis() - start); + } + + @Override + public FileIO io() { + return tableFileIO; + } + + @Override + public String metadataFileLocation(String filename) { + return metadataFileLocation(current(), filename); + } + + @Override + public LocationProvider locationProvider() { + return LocationProviders.locationsFor(current().location(), current().properties()); + } + public void doRefresh() { LOGGER.debug("doRefresh for tableIdentifier {}", tableIdentifier); // While doing refresh/commit protocols, we must fetch the fresh "passthrough" resolved @@ -1475,77 +1546,6 @@ protected String writeNewMetadata(TableMetadata metadata, int newVersion) { return newMetadataLocation.location(); } - @Override - public TableMetadata current() { - if (shouldRefresh) { - return refresh(); - } - return currentMetadata; - } - - @Override - public TableMetadata refresh() { - boolean currentMetadataWasAvailable = currentMetadata != null; - try { - doRefresh(); - } catch (NoSuchTableException e) { - if (currentMetadataWasAvailable) { - LOGGER.warn("Could not find the table during refresh, setting current metadata to null", e); - shouldRefresh = true; - } - - currentMetadata = null; - currentMetadataLocation = null; - version = -1; - throw e; - } - return current(); - } - - @Override - public void commit(TableMetadata base, TableMetadata metadata) { - // if the metadata is already out of date, reject it - if (base != current()) { - if (base != null) { - throw new CommitFailedException("Cannot commit: stale table metadata"); - } else { - // when current is non-null, the table exists. but when base is null, the commit is trying - // to create the table - throw new AlreadyExistsException("Table already exists: %s", tableName()); - } - } - // if the metadata is not changed, return early - if (base == metadata) { - LOGGER.info("Nothing to commit."); - return; - } - - long start = System.currentTimeMillis(); - doCommit(base, metadata); - CatalogUtil.deleteRemovedMetadataFiles(io(), base, metadata); - requestRefresh(); - - LOGGER.info( - "Successfully committed to table {} in {} ms", - tableName(), - System.currentTimeMillis() - start); - } - - @Override - public FileIO io() { - return tableFileIO; - } - - @Override - public String metadataFileLocation(String filename) { - return metadataFileLocation(current(), filename); - } - - @Override - public LocationProvider locationProvider() { - return LocationProviders.locationsFor(current().location(), current().properties()); - } - protected String tableName() { return fullTableName; } From 5f39622b61a6910b916cbe414b58f6c836a541b2 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 22 Apr 2025 10:54:30 -0700 Subject: [PATCH 03/12] view copy paste --- .../catalog/iceberg/IcebergCatalog.java | 178 +++++++++++++++++- 1 file changed, 174 insertions(+), 4 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index f346dbaf3c..0d601042e9 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -86,6 +86,7 @@ import org.apache.iceberg.view.ViewMetadata; import org.apache.iceberg.view.ViewMetadataParser; import org.apache.iceberg.view.ViewOperations; +import org.apache.iceberg.view.ViewProperties; import org.apache.iceberg.view.ViewUtil; import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.admin.model.StorageConfigInfo; @@ -1610,11 +1611,19 @@ private static int parseVersion(String metadataLocation) { } } - private class BasePolarisViewOperations extends BaseViewOperations { + private class BasePolarisViewOperations extends ViewOperations { private final TableIdentifier identifier; private final String fullViewName; private FileIO viewFileIO; + private static final String METADATA_FOLDER_NAME = "metadata"; + + private ViewMetadata currentMetadata = null; + private String currentMetadataLocation = null; + private boolean shouldRefresh = true; + private int version = -1; + + BasePolarisViewOperations(FileIO defaultFileIO, TableIdentifier identifier) { this.viewFileIO = defaultFileIO; this.identifier = identifier; @@ -1622,6 +1631,72 @@ private class BasePolarisViewOperations extends BaseViewOperations { } @Override + public ViewMetadata current() { + if (shouldRefresh) { + return refresh(); + } + + return currentMetadata; + } + + @Override + public ViewMetadata refresh() { + boolean currentMetadataWasAvailable = currentMetadata != null; + try { + doRefresh(); + } catch (NoSuchViewException e) { + if (currentMetadataWasAvailable) { + LOGGER.warn("Could not find the view during refresh, setting current metadata to null", e); + shouldRefresh = true; + } + + currentMetadata = null; + currentMetadataLocation = null; + version = -1; + throw e; + } + + return current(); + } + + @Override + @SuppressWarnings("ImmutablesReferenceEquality") + public void commit(ViewMetadata base, ViewMetadata metadata) { + // if the metadata is already out of date, reject it + if (base != current()) { + if (base != null) { + throw new CommitFailedException("Cannot commit: stale view metadata"); + } else { + // when current is non-null, the view exists. but when base is null, the commit is trying + // to create the view + throw new AlreadyExistsException("View already exists: %s", viewName()); + } + } + + // if the metadata is not changed, return early + if (base == metadata) { + LOGGER.info("Nothing to commit."); + return; + } + + long start = System.currentTimeMillis(); + doCommit(base, metadata); + requestRefresh(); + + LOGGER.info( + "Successfully committed to view {} in {} ms", + viewName(), + System.currentTimeMillis() - start); + } + + protected void requestRefresh() { + this.shouldRefresh = true; + } + + protected void disableRefresh() { + this.shouldRefresh = false; + } + public void doRefresh() { PolarisResolvedPathWrapper resolvedEntities = resolvedEntityView.getPassthroughResolvedPath( @@ -1668,7 +1743,6 @@ public void doRefresh() { } } - @Override public void doCommit(ViewMetadata base, ViewMetadata metadata) { // TODO: Maybe avoid writing metadata if there's definitely a transaction conflict LOGGER.debug("doCommit for view {} with base {}, metadata {}", identifier, base, metadata); @@ -1765,15 +1839,111 @@ public void doCommit(ViewMetadata base, ViewMetadata metadata) { } } - @Override + protected String writeNewMetadataIfRequired(ViewMetadata metadata) { + return null != metadata.metadataFileLocation() + ? metadata.metadataFileLocation() + : writeNewMetadata(metadata, version + 1); + } + + private String writeNewMetadata(ViewMetadata metadata, int newVersion) { + String newMetadataFilePath = newMetadataFilePath(metadata, newVersion); + OutputFile newMetadataLocation = io().newOutputFile(newMetadataFilePath); + + // write the new metadata + // use overwrite to avoid negative caching in S3. this is safe because the metadata location is + // always unique because it includes a UUID. + ViewMetadataParser.overwrite(metadata, newMetadataLocation); + + return newMetadataLocation.location(); + } + + private String newMetadataFilePath(ViewMetadata metadata, int newVersion) { + String codecName = + metadata + .properties() + .getOrDefault( + ViewProperties.METADATA_COMPRESSION, ViewProperties.METADATA_COMPRESSION_DEFAULT); + String fileExtension = TableMetadataParser.getFileExtension(codecName); + return metadataFileLocation( + metadata, + String.format(Locale.ROOT, "%05d-%s%s", newVersion, UUID.randomUUID(), fileExtension)); + } + + private String metadataFileLocation(ViewMetadata metadata, String filename) { + String metadataLocation = metadata.properties().get(ViewProperties.WRITE_METADATA_LOCATION); + + if (metadataLocation != null) { + return String.format("%s/%s", LocationUtil.stripTrailingSlash(metadataLocation), filename); + } else { + return String.format( + "%s/%s/%s", + LocationUtil.stripTrailingSlash(metadata.location()), METADATA_FOLDER_NAME, filename); + } + } + public FileIO io() { return viewFileIO; } - @Override protected String viewName() { return fullViewName; } + + protected String currentMetadataLocation() { + return currentMetadataLocation; + } + + protected int currentVersion() { + return version; + } + + protected void refreshFromMetadataLocation( + String newLocation, + Predicate shouldRetry, + int numRetries, + Function metadataLoader) { + if (!Objects.equal(currentMetadataLocation, newLocation)) { + LOGGER.info("Refreshing view metadata from new version: {}", newLocation); + + AtomicReference newMetadata = new AtomicReference<>(); + Tasks.foreach(newLocation) + .retry(numRetries) + .exponentialBackoff(100, 5000, 600000, 4.0 /* 100, 400, 1600, ... */) + .throwFailureWhenFinished() + .stopRetryOn(NotFoundException.class) // overridden if shouldRetry is non-null + .shouldRetryTest(shouldRetry) + .run(metadataLocation -> newMetadata.set(metadataLoader.apply(metadataLocation))); + + this.currentMetadata = newMetadata.get(); + this.currentMetadataLocation = newLocation; + this.version = parseVersion(newLocation); + } + + this.shouldRefresh = false; + } + + /** + * Parse the version from view metadata file name. + * + * @param metadataLocation view metadata file location + * @return version of the view metadata file in success case and -1 if the version is not parsable + * (as a sign that the metadata is not part of this catalog) + */ + private static int parseVersion(String metadataLocation) { + int versionStart = metadataLocation.lastIndexOf('/') + 1; // if '/' isn't found, this will be 0 + int versionEnd = metadataLocation.indexOf('-', versionStart); + if (versionEnd < 0) { + // found filesystem view's metadata + return -1; + } + + try { + return Integer.parseInt(metadataLocation.substring(versionStart, versionEnd)); + } catch (NumberFormatException e) { + LOGGER.warn("Unable to parse version from metadata location: {}", metadataLocation, e); + return -1; + } + } } private void validateMetadataFileInTableDir( From 1868f9f39a6eb90a925edb3e4a6667c1edeba8c7 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 22 Apr 2025 11:30:04 -0700 Subject: [PATCH 04/12] fixes, polish --- .../catalog/iceberg/IcebergCatalog.java | 277 ++++++++---------- .../iceberg/IcebergCatalogAdapter.java | 1 + 2 files changed, 122 insertions(+), 156 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index 0d601042e9..ff552ac55e 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -61,6 +61,7 @@ import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.SupportsNamespaces; import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.encryption.EncryptionManager; import org.apache.iceberg.exceptions.AlreadyExistsException; import org.apache.iceberg.exceptions.BadRequestException; import org.apache.iceberg.exceptions.CommitFailedException; @@ -81,7 +82,6 @@ import org.apache.iceberg.util.PropertyUtil; import org.apache.iceberg.util.Tasks; import org.apache.iceberg.view.BaseMetastoreViewCatalog; -import org.apache.iceberg.view.BaseViewOperations; import org.apache.iceberg.view.ViewBuilder; import org.apache.iceberg.view.ViewMetadata; import org.apache.iceberg.view.ViewMetadataParser; @@ -1202,18 +1202,12 @@ public ViewBuilder withLocation(String newLocation) { } } - private class BasePolarisTableOperations implements TableOperations { + private class BasePolarisTableOperations extends PolarisOperationsBase + implements TableOperations { private final TableIdentifier tableIdentifier; private final String fullTableName; private FileIO tableFileIO; - private static final String METADATA_FOLDER_NAME = "metadata"; - - private TableMetadata currentMetadata = null; - private String currentMetadataLocation = null; - private boolean shouldRefresh = true; - private int version = -1; - BasePolarisTableOperations(FileIO defaultFileIO, TableIdentifier tableIdentifier) { LOGGER.debug("new BasePolarisTableOperations for {}", tableIdentifier); this.tableIdentifier = tableIdentifier; @@ -1236,7 +1230,8 @@ public TableMetadata refresh() { doRefresh(); } catch (NoSuchTableException e) { if (currentMetadataWasAvailable) { - LOGGER.warn("Could not find the table during refresh, setting current metadata to null", e); + LOGGER.warn( + "Could not find the table during refresh, setting current metadata to null", e); shouldRefresh = true; } @@ -1257,7 +1252,7 @@ public void commit(TableMetadata base, TableMetadata metadata) { } else { // when current is non-null, the table exists. but when base is null, the commit is trying // to create the table - throw new AlreadyExistsException("Table already exists: %s", tableName()); + throw new AlreadyExistsException("Table already exists: %s", fullTableName); } } // if the metadata is not changed, return early @@ -1273,7 +1268,7 @@ public void commit(TableMetadata base, TableMetadata metadata) { LOGGER.info( "Successfully committed to table {} in {} ms", - tableName(), + fullTableName, System.currentTimeMillis() - start); } @@ -1339,40 +1334,6 @@ public void doRefresh() { } } - protected void refreshFromMetadataLocation( - String newLocation, - Predicate shouldRetry, - int numRetries, - Function metadataLoader) { - // use null-safe equality check because new tables have a null metadata location - if (!Objects.equal(currentMetadataLocation, newLocation)) { - LOGGER.info("Refreshing table metadata from new version: {}", newLocation); - - AtomicReference newMetadata = new AtomicReference<>(); - Tasks.foreach(newLocation) - .retry(numRetries) - .exponentialBackoff(100, 5000, 600000, 4.0 /* 100, 400, 1600, ... */) - .throwFailureWhenFinished() - .stopRetryOn(NotFoundException.class) // overridden if shouldRetry is non-null - .shouldRetryTest(shouldRetry) - .run(metadataLocation -> newMetadata.set(metadataLoader.apply(metadataLocation))); - - String newUUID = newMetadata.get().uuid(); - if (currentMetadata != null && currentMetadata.uuid() != null && newUUID != null) { - Preconditions.checkState( - newUUID.equals(currentMetadata.uuid()), - "Table UUID does not match: current=%s != refreshed=%s", - currentMetadata.uuid(), - newUUID); - } - - this.currentMetadata = newMetadata.get(); - this.currentMetadataLocation = newLocation; - this.version = parseVersion(newLocation); - } - this.shouldRefresh = false; - } - public void doCommit(TableMetadata base, TableMetadata metadata) { LOGGER.debug( "doCommit for table {} with base {}, metadata {}", tableIdentifier, base, metadata); @@ -1498,11 +1459,11 @@ public void doCommit(TableMetadata base, TableMetadata metadata) { } if (!Objects.equal(existingLocation, oldLocation)) { if (null == base) { - throw new AlreadyExistsException("Table already exists: %s", tableName()); + throw new AlreadyExistsException("Table already exists: %s", fullTableName); } if (null == existingLocation) { - throw new NoSuchTableException("Table does not exist: %s", tableName()); + throw new NoSuchTableException("Table does not exist: %s", fullTableName); } throw new CommitFailedException( @@ -1517,22 +1478,58 @@ public void doCommit(TableMetadata base, TableMetadata metadata) { } } - public int currentVersion() { - return version; - } + @Override + public TableOperations temp(TableMetadata uncommittedMetadata) { + return new TableOperations() { + @Override + public TableMetadata current() { + return uncommittedMetadata; + } - protected void requestRefresh() { - this.shouldRefresh = true; - } + @Override + public TableMetadata refresh() { + throw new UnsupportedOperationException( + "Cannot call refresh on temporary table operations"); + } - protected void disableRefresh() { - this.shouldRefresh = false; + @Override + public void commit(TableMetadata base, TableMetadata metadata) { + throw new UnsupportedOperationException("Cannot call commit on temporary table operations"); + } + + @Override + public String metadataFileLocation(String fileName) { + return BasePolarisTableOperations.this.metadataFileLocation( + uncommittedMetadata, fileName); + } + + @Override + public LocationProvider locationProvider() { + return LocationProviders.locationsFor( + uncommittedMetadata.location(), uncommittedMetadata.properties()); + } + + @Override + public FileIO io() { + return BasePolarisTableOperations.this.io(); + } + + @Override + public EncryptionManager encryption() { + return BasePolarisTableOperations.this.encryption(); + } + + @Override + public long newSnapshotId() { + return BasePolarisTableOperations.this.newSnapshotId(); + } + }; } protected String writeNewMetadataIfRequired(boolean newTable, TableMetadata metadata) { return newTable && metadata.metadataFileLocation() != null ? metadata.metadataFileLocation() - : writeNewMetadata(metadata, currentVersion() + 1); + : writeNewMetadata(metadata, version + 1); } protected String writeNewMetadata(TableMetadata metadata, int newVersion) { @@ -1540,17 +1537,14 @@ protected String writeNewMetadata(TableMetadata metadata, int newVersion) { OutputFile newMetadataLocation = io().newOutputFile(newTableMetadataFilePath); // write the new metadata - // use overwrite to avoid negative caching in S3. this is safe because the metadata location is + // use overwrite to avoid negative caching in S3. this is safe because the metadata location + // is // always unique because it includes a UUID. TableMetadataParser.overwrite(metadata, newMetadataLocation); return newMetadataLocation.location(); } - protected String tableName() { - return fullTableName; - } - private String metadataFileLocation(TableMetadata metadata, String filename) { String metadataLocation = metadata.properties().get(TableProperties.WRITE_METADATA_LOCATION); @@ -1561,22 +1555,6 @@ private String metadataFileLocation(TableMetadata metadata, String filename) { } } - /** - * Validate if the new metadata location is the current metadata location or present within - * previous metadata files. - * - * @param newMetadataLocation newly written metadata location - * @return true if the new metadata location is the current metadata location or present within - * previous metadata files. - */ - private boolean checkCurrentMetadataLocation(String newMetadataLocation) { - TableMetadata metadata = refresh(); - String currentMetadataFileLocation = metadata.metadataFileLocation(); - return currentMetadataFileLocation.equals(newMetadataLocation) - || metadata.previousFiles().stream() - .anyMatch(log -> log.file().equals(newMetadataLocation)); - } - private String newTableMetadataFilePath(TableMetadata meta, int newVersion) { String codecName = meta.property( @@ -1586,44 +1564,14 @@ private String newTableMetadataFilePath(TableMetadata meta, int newVersion) { meta, String.format(Locale.ROOT, "%05d-%s%s", newVersion, UUID.randomUUID(), fileExtension)); } - - /** - * Parse the version from table metadata file name. - * - * @param metadataLocation table metadata file location - * @return version of the table metadata file in success case and -1 if the version is not - * parsable (as a sign that the metadata is not part of this catalog) - */ - private static int parseVersion(String metadataLocation) { - int versionStart = metadataLocation.lastIndexOf('/') + 1; // if '/' isn't found, this will be 0 - int versionEnd = metadataLocation.indexOf('-', versionStart); - if (versionEnd < 0) { - // found filesystem table's metadata - return -1; - } - - try { - return Integer.parseInt(metadataLocation.substring(versionStart, versionEnd)); - } catch (NumberFormatException e) { - LOGGER.warn("Unable to parse version from metadata location: {}", metadataLocation, e); - return -1; - } - } } - private class BasePolarisViewOperations extends ViewOperations { + private class BasePolarisViewOperations extends PolarisOperationsBase + implements ViewOperations { private final TableIdentifier identifier; private final String fullViewName; private FileIO viewFileIO; - private static final String METADATA_FOLDER_NAME = "metadata"; - - private ViewMetadata currentMetadata = null; - private String currentMetadataLocation = null; - private boolean shouldRefresh = true; - private int version = -1; - - BasePolarisViewOperations(FileIO defaultFileIO, TableIdentifier identifier) { this.viewFileIO = defaultFileIO; this.identifier = identifier; @@ -1646,7 +1594,8 @@ public ViewMetadata refresh() { doRefresh(); } catch (NoSuchViewException e) { if (currentMetadataWasAvailable) { - LOGGER.warn("Could not find the view during refresh, setting current metadata to null", e); + LOGGER.warn( + "Could not find the view during refresh, setting current metadata to null", e); shouldRefresh = true; } @@ -1689,14 +1638,6 @@ public void commit(ViewMetadata base, ViewMetadata metadata) { System.currentTimeMillis() - start); } - protected void requestRefresh() { - this.shouldRefresh = true; - } - - protected void disableRefresh() { - this.shouldRefresh = false; - } - public void doRefresh() { PolarisResolvedPathWrapper resolvedEntities = resolvedEntityView.getPassthroughResolvedPath( @@ -1798,7 +1739,7 @@ public void doCommit(ViewMetadata base, ViewMetadata metadata) { Set.of(PolarisStorageActions.READ, PolarisStorageActions.WRITE)); String newLocation = writeNewMetadataIfRequired(metadata); - String oldLocation = base == null ? null : currentMetadataLocation(); + String oldLocation = base == null ? null : currentMetadataLocation; IcebergTableLikeEntity entity = IcebergTableLikeEntity.of( @@ -1850,7 +1791,8 @@ private String writeNewMetadata(ViewMetadata metadata, int newVersion) { OutputFile newMetadataLocation = io().newOutputFile(newMetadataFilePath); // write the new metadata - // use overwrite to avoid negative caching in S3. this is safe because the metadata location is + // use overwrite to avoid negative caching in S3. this is safe because the metadata location + // is // always unique because it includes a UUID. ViewMetadataParser.overwrite(metadata, newMetadataLocation); @@ -1871,7 +1813,6 @@ private String newMetadataFilePath(ViewMetadata metadata, int newVersion) { private String metadataFileLocation(ViewMetadata metadata, String filename) { String metadataLocation = metadata.properties().get(ViewProperties.WRITE_METADATA_LOCATION); - if (metadataLocation != null) { return String.format("%s/%s", LocationUtil.stripTrailingSlash(metadataLocation), filename); } else { @@ -1888,24 +1829,59 @@ public FileIO io() { protected String viewName() { return fullViewName; } + } + + private abstract static class PolarisOperationsBase { + + protected static final String METADATA_FOLDER_NAME = "metadata"; + + protected T currentMetadata = null; + protected String currentMetadataLocation = null; + protected boolean shouldRefresh = true; + protected int version = -1; + + protected void requestRefresh() { + this.shouldRefresh = true; + } - protected String currentMetadataLocation() { - return currentMetadataLocation; + protected void disableRefresh() { + this.shouldRefresh = false; } - protected int currentVersion() { - return version; + /** + * Parse the version from table/view metadata file name. + * + * @param metadataLocation table/view metadata file location + * @return version of the table/view metadata file in success case and -1 if the version is not + * parsable (as a sign that the metadata is not part of this catalog) + */ + protected int parseVersion(String metadataLocation) { + int versionStart = + metadataLocation.lastIndexOf('/') + 1; // if '/' isn't found, this will be 0 + int versionEnd = metadataLocation.indexOf('-', versionStart); + if (versionEnd < 0) { + // found filesystem object's metadata + return -1; + } + + try { + return Integer.parseInt(metadataLocation.substring(versionStart, versionEnd)); + } catch (NumberFormatException e) { + LOGGER.warn("Unable to parse version from metadata location: {}", metadataLocation, e); + return -1; + } } protected void refreshFromMetadataLocation( String newLocation, Predicate shouldRetry, int numRetries, - Function metadataLoader) { + Function metadataLoader) { + // use null-safe equality check because new tables have a null metadata location if (!Objects.equal(currentMetadataLocation, newLocation)) { - LOGGER.info("Refreshing view metadata from new version: {}", newLocation); + LOGGER.info("Refreshing table metadata from new version: {}", newLocation); - AtomicReference newMetadata = new AtomicReference<>(); + AtomicReference newMetadata = new AtomicReference<>(); Tasks.foreach(newLocation) .retry(numRetries) .exponentialBackoff(100, 5000, 600000, 4.0 /* 100, 400, 1600, ... */) @@ -1914,36 +1890,25 @@ protected void refreshFromMetadataLocation( .shouldRetryTest(shouldRetry) .run(metadataLocation -> newMetadata.set(metadataLoader.apply(metadataLocation))); + if (newMetadata.get() instanceof TableMetadata tableMetadata) { + if (currentMetadata instanceof TableMetadata currentTableMetadata) { + String newUUID = tableMetadata.uuid(); + if (currentMetadata != null && currentTableMetadata.uuid() != null && newUUID != null) { + Preconditions.checkState( + newUUID.equals(currentTableMetadata.uuid()), + "Table UUID does not match: current=%s != refreshed=%s", + currentTableMetadata.uuid(), + newUUID); + } + } + } + this.currentMetadata = newMetadata.get(); this.currentMetadataLocation = newLocation; this.version = parseVersion(newLocation); } - this.shouldRefresh = false; } - - /** - * Parse the version from view metadata file name. - * - * @param metadataLocation view metadata file location - * @return version of the view metadata file in success case and -1 if the version is not parsable - * (as a sign that the metadata is not part of this catalog) - */ - private static int parseVersion(String metadataLocation) { - int versionStart = metadataLocation.lastIndexOf('/') + 1; // if '/' isn't found, this will be 0 - int versionEnd = metadataLocation.indexOf('-', versionStart); - if (versionEnd < 0) { - // found filesystem view's metadata - return -1; - } - - try { - return Integer.parseInt(metadataLocation.substring(versionStart, versionEnd)); - } catch (NumberFormatException e) { - LOGGER.warn("Unable to parse version from metadata location: {}", metadataLocation, e); - return -1; - } - } } private void validateMetadataFileInTableDir( @@ -1956,9 +1921,9 @@ private void validateMetadataFileInTableDir( polarisCallContext, FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION); if (!allowEscape && !polarisCallContext - .getConfigurationStore() - .getConfiguration( - polarisCallContext, FeatureConfiguration.ALLOW_EXTERNAL_METADATA_FILE_LOCATION)) { + .getConfigurationStore() + .getConfiguration( + polarisCallContext, FeatureConfiguration.ALLOW_EXTERNAL_METADATA_FILE_LOCATION)) { LOGGER.debug( "Validating base location {} for table {} in metadata file {}", metadata.location(), diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java index 6895351395..60b4187624 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java @@ -369,6 +369,7 @@ public Response loadTable( String namespace, String table, String accessDelegationMode, + String ifNoneMatchString, // TODO remove this String snapshots, RealmContext realmContext, SecurityContext securityContext) { From b6a8f41418580e2a9a1ece7100c40fc8596489d0 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 22 Apr 2025 11:31:51 -0700 Subject: [PATCH 05/12] stable --- .../polaris/service/catalog/iceberg/IcebergCatalog.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index ff552ac55e..e19eba6dd4 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -47,7 +47,6 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import org.apache.commons.lang3.exception.ExceptionUtils; -import org.apache.iceberg.BaseMetastoreTableOperations; import org.apache.iceberg.BaseTable; import org.apache.iceberg.CatalogProperties; import org.apache.iceberg.CatalogUtil; @@ -1494,7 +1493,8 @@ public TableMetadata refresh() { @Override public void commit(TableMetadata base, TableMetadata metadata) { - throw new UnsupportedOperationException("Cannot call commit on temporary table operations"); + throw new UnsupportedOperationException( + "Cannot call commit on temporary table operations"); } @Override From fa87494df001fa6b94288883ce170347ce0d23ba Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 22 Apr 2025 11:37:35 -0700 Subject: [PATCH 06/12] yank --- .../polaris/service/catalog/iceberg/IcebergCatalogAdapter.java | 1 - 1 file changed, 1 deletion(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java index 60b4187624..6895351395 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java @@ -369,7 +369,6 @@ public Response loadTable( String namespace, String table, String accessDelegationMode, - String ifNoneMatchString, // TODO remove this String snapshots, RealmContext realmContext, SecurityContext securityContext) { From 625051b542d80dfc379b74773917affb1296a741 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Wed, 23 Apr 2025 16:50:33 -0700 Subject: [PATCH 07/12] CODE_COPIED_TO_POLARIS comments --- .../service/catalog/iceberg/IcebergCatalog.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index e19eba6dd4..2759183902 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -1201,6 +1201,11 @@ public ViewBuilder withLocation(String newLocation) { } } + /** + * An implementation of {@link TableOperations} that integrates with {@link IcebergCatalog}. + * Much of this code was originally copied from {@link org.apache.iceberg.BaseMetastoreTableOperations}. + * COPIED_COPIED_TO_POLARIS + */ private class BasePolarisTableOperations extends PolarisOperationsBase implements TableOperations { private final TableIdentifier tableIdentifier; @@ -1566,6 +1571,11 @@ private String newTableMetadataFilePath(TableMetadata meta, int newVersion) { } } + /** + * An implementation of {@link ViewOperations} that integrates with {@link IcebergCatalog}. + * Much of this code was originally copied from {@link org.apache.iceberg.view.BaseViewOperations}. + * COPIED_COPIED_TO_POLARIS + */ private class BasePolarisViewOperations extends PolarisOperationsBase implements ViewOperations { private final TableIdentifier identifier; @@ -1831,6 +1841,11 @@ protected String viewName() { } } + /** + * An ABC for {@link BasePolarisTableOperations} and {@link BasePolarisViewOperations}. + * Much of this code was originally copied from {@link org.apache.iceberg.BaseMetastoreTableOperations}. + * COPIED_COPIED_TO_POLARIS + */ private abstract static class PolarisOperationsBase { protected static final String METADATA_FOLDER_NAME = "metadata"; From b6b0cf911ce93ea87e23837fac3221ea215c8029 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Wed, 23 Apr 2025 16:50:40 -0700 Subject: [PATCH 08/12] autolint --- .../service/catalog/iceberg/IcebergCatalog.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index 2759183902..1bbf18213f 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -1202,9 +1202,9 @@ public ViewBuilder withLocation(String newLocation) { } /** - * An implementation of {@link TableOperations} that integrates with {@link IcebergCatalog}. - * Much of this code was originally copied from {@link org.apache.iceberg.BaseMetastoreTableOperations}. - * COPIED_COPIED_TO_POLARIS + * An implementation of {@link TableOperations} that integrates with {@link IcebergCatalog}. Much + * of this code was originally copied from {@link + * org.apache.iceberg.BaseMetastoreTableOperations}. COPIED_COPIED_TO_POLARIS */ private class BasePolarisTableOperations extends PolarisOperationsBase implements TableOperations { @@ -1572,8 +1572,8 @@ private String newTableMetadataFilePath(TableMetadata meta, int newVersion) { } /** - * An implementation of {@link ViewOperations} that integrates with {@link IcebergCatalog}. - * Much of this code was originally copied from {@link org.apache.iceberg.view.BaseViewOperations}. + * An implementation of {@link ViewOperations} that integrates with {@link IcebergCatalog}. Much + * of this code was originally copied from {@link org.apache.iceberg.view.BaseViewOperations}. * COPIED_COPIED_TO_POLARIS */ private class BasePolarisViewOperations extends PolarisOperationsBase @@ -1842,8 +1842,8 @@ protected String viewName() { } /** - * An ABC for {@link BasePolarisTableOperations} and {@link BasePolarisViewOperations}. - * Much of this code was originally copied from {@link org.apache.iceberg.BaseMetastoreTableOperations}. + * An ABC for {@link BasePolarisTableOperations} and {@link BasePolarisViewOperations}. Much of + * this code was originally copied from {@link org.apache.iceberg.BaseMetastoreTableOperations}. * COPIED_COPIED_TO_POLARIS */ private abstract static class PolarisOperationsBase { From 03c3a6ad6640db3dede99661ef51411382ac9846 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 24 Apr 2025 11:37:24 -0700 Subject: [PATCH 09/12] update license --- LICENSE | 2 ++ 1 file changed, 2 insertions(+) diff --git a/LICENSE b/LICENSE index e9c023b306..f7e0996e33 100644 --- a/LICENSE +++ b/LICENSE @@ -218,6 +218,8 @@ This product includes code from Apache Iceberg. * spec/iceberg-rest-catalog-open-api.yaml * spec/polaris-catalog-apis/oauth-tokens-api.yaml * integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +* integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java +* service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java Copyright: Copyright 2017-2025 The Apache Software Foundation Home page: https://iceberg.apache.org From 3455df6b29a81b2c746adc85b8dbe1dfc95184c0 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 24 Apr 2025 11:37:43 -0700 Subject: [PATCH 10/12] typofix --- LICENSE | 1 - 1 file changed, 1 deletion(-) diff --git a/LICENSE b/LICENSE index f7e0996e33..7b965b4a5d 100644 --- a/LICENSE +++ b/LICENSE @@ -218,7 +218,6 @@ This product includes code from Apache Iceberg. * spec/iceberg-rest-catalog-open-api.yaml * spec/polaris-catalog-apis/oauth-tokens-api.yaml * integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java -* integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java * service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java Copyright: Copyright 2017-2025 The Apache Software Foundation From e539370105515e3e2490832e64bec3f0efd1dcc2 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 24 Apr 2025 11:39:29 -0700 Subject: [PATCH 11/12] update comments --- .../polaris/service/catalog/iceberg/IcebergCatalog.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index 1bbf18213f..0fd0097682 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -1203,8 +1203,8 @@ public ViewBuilder withLocation(String newLocation) { /** * An implementation of {@link TableOperations} that integrates with {@link IcebergCatalog}. Much - * of this code was originally copied from {@link - * org.apache.iceberg.BaseMetastoreTableOperations}. COPIED_COPIED_TO_POLARIS + * of this code was originally copied from {@link org.apache.iceberg.BaseMetastoreTableOperations}. + * CODE_COPIED_TO_POLARIS From Apache Iceberg Version: 1.8 */ private class BasePolarisTableOperations extends PolarisOperationsBase implements TableOperations { @@ -1574,7 +1574,7 @@ private String newTableMetadataFilePath(TableMetadata meta, int newVersion) { /** * An implementation of {@link ViewOperations} that integrates with {@link IcebergCatalog}. Much * of this code was originally copied from {@link org.apache.iceberg.view.BaseViewOperations}. - * COPIED_COPIED_TO_POLARIS + * CODE_COPIED_TO_POLARIS From Apache Iceberg Version: 1.8 */ private class BasePolarisViewOperations extends PolarisOperationsBase implements ViewOperations { @@ -1844,7 +1844,7 @@ protected String viewName() { /** * An ABC for {@link BasePolarisTableOperations} and {@link BasePolarisViewOperations}. Much of * this code was originally copied from {@link org.apache.iceberg.BaseMetastoreTableOperations}. - * COPIED_COPIED_TO_POLARIS + * CODE_COPIED_TO_POLARIS From Apache Iceberg Version: 1.8 */ private abstract static class PolarisOperationsBase { From 0c9cb579697c4cc9e23c3f9c92812587f78956a1 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Thu, 24 Apr 2025 11:39:31 -0700 Subject: [PATCH 12/12] autolint --- .../polaris/service/catalog/iceberg/IcebergCatalog.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index 0fd0097682..ae34be9285 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -1203,8 +1203,9 @@ public ViewBuilder withLocation(String newLocation) { /** * An implementation of {@link TableOperations} that integrates with {@link IcebergCatalog}. Much - * of this code was originally copied from {@link org.apache.iceberg.BaseMetastoreTableOperations}. - * CODE_COPIED_TO_POLARIS From Apache Iceberg Version: 1.8 + * of this code was originally copied from {@link + * org.apache.iceberg.BaseMetastoreTableOperations}. CODE_COPIED_TO_POLARIS From Apache Iceberg + * Version: 1.8 */ private class BasePolarisTableOperations extends PolarisOperationsBase implements TableOperations {