You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@polaris.apache.org by ad...@apache.org on 2026/08/05 09:02:49 UTC
(polaris) branch main updated: Remove loadTable convenience overloads from IcebergCatalogHandler (part of #4709) (#5230)
This is an automated email from the ASF dual-hosted git repository.
adutra pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/polaris.git
The following commit(s) were added to refs/heads/main by this push:
new 57735645b8 Remove loadTable convenience overloads from IcebergCatalogHandler (part of #4709) (#5230)
57735645b8 is described below
commit 57735645b81f741f4f39f6be11a1dbaa06f270fd
Author: Paras Mishra <13...@users.noreply.github.com>
AuthorDate: Wed Aug 5 14:32:44 2026 +0530
Remove loadTable convenience overloads from IcebergCatalogHandler (part of #4709) (#5230)
---
.../catalog/iceberg/IcebergCatalogHandler.java | 60 +-------
.../AbstractIcebergCatalogHandlerAuthzTest.java | 157 ++++++++++++++++++---
.../iceberg/EntityIdempotencyUpdateTableTest.java | 7 +-
3 files changed, 147 insertions(+), 77 deletions(-)
diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java
index 4a014a6f64..d127bc0bb7 100644
--- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java
+++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java
@@ -867,66 +867,10 @@ public abstract class IcebergCatalogHandler extends CatalogHandler implements Au
return IcebergTableLikeEntity.of(leaf);
}
- public LoadTableResponse loadTable(TableIdentifier tableIdentifier, String snapshots) {
- return loadTableIfStale(tableIdentifier, null, snapshots).get();
- }
-
- /**
- * Attempt to perform a loadTable operation only when the specified set of eTags do not match the
- * current state of the table metadata.
- *
- * @param tableIdentifier The identifier of the table to load
- * @param ifNoneMatch set of entity-tags to check the metadata against for staleness
- * @param snapshots
- * @return {@link Optional#empty()} if the ETag is current, an {@link Optional} containing the
- * load table response, otherwise
- */
- public Optional<LoadTableResponse> loadTableIfStale(
- TableIdentifier tableIdentifier, IfNoneMatch ifNoneMatch, String snapshots) {
- return loadTable(
- tableIdentifier,
- snapshots,
- ifNoneMatch,
- EnumSet.noneOf(AccessDelegationMode.class),
- Optional.empty());
- }
-
- public LoadTableResponse loadTableWithAccessDelegation(
- TableIdentifier tableIdentifier,
- String snapshots,
- Optional<String> refreshCredentialsEndpoint) {
- return loadTableWithAccessDelegationIfStale(
- tableIdentifier, null, snapshots, refreshCredentialsEndpoint)
- .get();
- }
-
- /**
- * Attempt to perform a loadTable operation with access delegation only when the if none of the
- * provided eTags match the current state of the table metadata.
- *
- * @param tableIdentifier The identifier of the table to load
- * @param ifNoneMatch set of entity-tags to check the metadata against for staleness
- * @param snapshots
- * @return {@link Optional#empty()} if the ETag is current, an {@link Optional} containing the
- * load table response, otherwise
- */
- public Optional<LoadTableResponse> loadTableWithAccessDelegationIfStale(
- TableIdentifier tableIdentifier,
- IfNoneMatch ifNoneMatch,
- String snapshots,
- Optional<String> refreshCredentialsEndpoint) {
- return loadTable(
- tableIdentifier,
- snapshots,
- ifNoneMatch,
- EnumSet.of(VENDED_CREDENTIALS),
- refreshCredentialsEndpoint);
- }
-
/**
* Vend credentials for a table using location data from entity internal properties, avoiding a
- * full table metadata read from object storage. Falls back to the standard
- * loadTableWithAccessDelegation path if the entity lacks the required location properties.
+ * full table metadata read from object storage. Falls back to the full loadTable path if the
+ * entity lacks the required location properties.
*/
public ImmutableLoadCredentialsResponse loadCredentials(
TableIdentifier tableIdentifier, Optional<String> refreshCredentialsEndpoint) {
diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractIcebergCatalogHandlerAuthzTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractIcebergCatalogHandlerAuthzTest.java
index 62e04a95df..00261faa3a 100644
--- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractIcebergCatalogHandlerAuthzTest.java
+++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractIcebergCatalogHandlerAuthzTest.java
@@ -613,7 +613,16 @@ public abstract class AbstractIcebergCatalogHandlerAuthzTest extends PolarisAuth
// To get a handy metadata file we can use one from another table.
// to avoid overlapping directories, drop the original table and recreate it via registerTable
- String metadataLocation = newHandler().loadTable(TABLE_NS1_1, "all").metadataLocation();
+ String metadataLocation =
+ newHandler()
+ .loadTable(
+ TABLE_NS1_1,
+ "all",
+ null,
+ EnumSet.noneOf(AccessDelegationMode.class),
+ Optional.empty())
+ .get()
+ .metadataLocation();
newHandler(Set.of(PRINCIPAL_ROLE2)).dropTableWithoutPurge(TABLE_NS1_1);
RegisterTableRequest registerRequest =
@@ -674,7 +683,15 @@ public abstract class AbstractIcebergCatalogHandlerAuthzTest extends PolarisAuth
NS2, createSourceRequest, EnumSet.noneOf(AccessDelegationMode.class), Optional.empty());
final String metadataLocation =
- newHandler(Set.of(PRINCIPAL_ROLE2)).loadTable(sourceTable, "all").metadataLocation();
+ newHandler(Set.of(PRINCIPAL_ROLE2))
+ .loadTable(
+ sourceTable,
+ "all",
+ null,
+ EnumSet.noneOf(AccessDelegationMode.class),
+ Optional.empty())
+ .get()
+ .metadataLocation();
newHandler(Set.of(PRINCIPAL_ROLE2)).dropTableWithoutPurge(sourceTable);
final RegisterTableRequest registerRequest =
@@ -738,7 +755,15 @@ public abstract class AbstractIcebergCatalogHandlerAuthzTest extends PolarisAuth
.grantPrivilegeOnCatalogToRole(
CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_READ_PROPERTIES));
final String metadataLocation =
- newHandler(Set.of(PRINCIPAL_ROLE2)).loadTable(TABLE_NS1_1, "all").metadataLocation();
+ newHandler(Set.of(PRINCIPAL_ROLE2))
+ .loadTable(
+ TABLE_NS1_1,
+ "all",
+ null,
+ EnumSet.noneOf(AccessDelegationMode.class),
+ Optional.empty())
+ .get()
+ .metadataLocation();
// Mock RegisterTableRequest with overwrite=true
final RegisterTableRequest registerRequest = Mockito.mock(RegisterTableRequest.class);
@@ -783,7 +808,16 @@ public abstract class AbstractIcebergCatalogHandlerAuthzTest extends PolarisAuth
.grantPrivilegeOnCatalogToRole(
CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_READ_PROPERTIES));
- String metadataLocation = newHandler().loadTable(TABLE_NS1_1, "all").metadataLocation();
+ String metadataLocation =
+ newHandler()
+ .loadTable(
+ TABLE_NS1_1,
+ "all",
+ null,
+ EnumSet.noneOf(AccessDelegationMode.class),
+ Optional.empty())
+ .get()
+ .metadataLocation();
newHandler(Set.of(PRINCIPAL_ROLE2)).dropTableWithoutPurge(TABLE_NS1_1);
RegisterTableRequest registerRequest =
@@ -835,7 +869,15 @@ public abstract class AbstractIcebergCatalogHandlerAuthzTest extends PolarisAuth
.grantPrivilegeOnCatalogToRole(
CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_READ_PROPERTIES));
String metadataLocation =
- newHandler(Set.of(PRINCIPAL_ROLE2)).loadTable(TABLE_NS1_1, "all").metadataLocation();
+ newHandler(Set.of(PRINCIPAL_ROLE2))
+ .loadTable(
+ TABLE_NS1_1,
+ "all",
+ null,
+ EnumSet.noneOf(AccessDelegationMode.class),
+ Optional.empty())
+ .get()
+ .metadataLocation();
RegisterTableRequest registerRequest = Mockito.mock(RegisterTableRequest.class);
Mockito.when(registerRequest.name()).thenReturn(TABLE_NS1_1.name());
@@ -887,7 +929,16 @@ public abstract class AbstractIcebergCatalogHandlerAuthzTest extends PolarisAuth
.grantPrivilegeOnCatalogToRole(
CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_READ_PROPERTIES));
- String metadataLocation = newHandler().loadTable(TABLE_NS1_1, "all").metadataLocation();
+ String metadataLocation =
+ newHandler()
+ .loadTable(
+ TABLE_NS1_1,
+ "all",
+ null,
+ EnumSet.noneOf(AccessDelegationMode.class),
+ Optional.empty())
+ .get()
+ .metadataLocation();
newHandler(Set.of(PRINCIPAL_ROLE2)).dropTableWithoutPurge(TABLE_NS1_1);
RegisterTableRequest registerRequest =
@@ -927,7 +978,16 @@ public abstract class AbstractIcebergCatalogHandlerAuthzTest extends PolarisAuth
.grantPrivilegeOnCatalogToRole(
CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_READ_PROPERTIES));
- String metadataLocation = newHandler().loadTable(TABLE_NS1_1, "all").metadataLocation();
+ String metadataLocation =
+ newHandler()
+ .loadTable(
+ TABLE_NS1_1,
+ "all",
+ null,
+ EnumSet.noneOf(AccessDelegationMode.class),
+ Optional.empty())
+ .get()
+ .metadataLocation();
RegisterTableRequest registerRequest =
ImmutableRegisterTableRequest.builder()
@@ -967,7 +1027,16 @@ public abstract class AbstractIcebergCatalogHandlerAuthzTest extends PolarisAuth
.grantPrivilegeOnCatalogToRole(
CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_READ_PROPERTIES));
- String metadataLocation = newHandler().loadTable(TABLE_NS1_1, "all").metadataLocation();
+ String metadataLocation =
+ newHandler()
+ .loadTable(
+ TABLE_NS1_1,
+ "all",
+ null,
+ EnumSet.noneOf(AccessDelegationMode.class),
+ Optional.empty())
+ .get()
+ .metadataLocation();
newHandler(Set.of(PRINCIPAL_ROLE2)).dropTableWithoutPurge(TABLE_NS1_1);
RegisterTableRequest registerRequest =
@@ -1009,7 +1078,16 @@ public abstract class AbstractIcebergCatalogHandlerAuthzTest extends PolarisAuth
.grantPrivilegeOnCatalogToRole(
CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_READ_PROPERTIES));
- String metadataLocation = newHandler().loadTable(TABLE_NS1_1, "all").metadataLocation();
+ String metadataLocation =
+ newHandler()
+ .loadTable(
+ TABLE_NS1_1,
+ "all",
+ null,
+ EnumSet.noneOf(AccessDelegationMode.class),
+ Optional.empty())
+ .get()
+ .metadataLocation();
RegisterTableRequest registerRequest =
ImmutableRegisterTableRequest.builder()
@@ -1050,7 +1128,15 @@ public abstract class AbstractIcebergCatalogHandlerAuthzTest extends PolarisAuth
@TestFactory
Stream<DynamicNode> testLoadTablePrivileges() {
return authzTestsBuilder("loadTable")
- .action(() -> newHandler().loadTable(TABLE_NS1A_2, "all"))
+ .action(
+ () ->
+ newHandler()
+ .loadTable(
+ TABLE_NS1A_2,
+ "all",
+ null,
+ EnumSet.noneOf(AccessDelegationMode.class),
+ Optional.empty()))
.shouldPassWith(PolarisPrivilege.TABLE_READ_PROPERTIES)
.shouldPassWith(PolarisPrivilege.TABLE_WRITE_PROPERTIES)
.shouldPassWith(PolarisPrivilege.TABLE_READ_DATA)
@@ -1067,7 +1153,12 @@ public abstract class AbstractIcebergCatalogHandlerAuthzTest extends PolarisAuth
.action(
() ->
newHandler()
- .loadTableIfStale(TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all"))
+ .loadTable(
+ TABLE_NS1A_2,
+ "all",
+ IfNoneMatch.fromHeader("W/\"0:0\""),
+ EnumSet.noneOf(AccessDelegationMode.class),
+ Optional.empty()))
.shouldPassWith(PolarisPrivilege.TABLE_READ_PROPERTIES)
.shouldPassWith(PolarisPrivilege.TABLE_WRITE_PROPERTIES)
.shouldPassWith(PolarisPrivilege.TABLE_READ_DATA)
@@ -1082,7 +1173,14 @@ public abstract class AbstractIcebergCatalogHandlerAuthzTest extends PolarisAuth
Stream<DynamicNode> testLoadTableWithReadAccessDelegationPrivileges() {
return authzTestsBuilder("loadTableWithAccessDelegation")
.action(
- () -> newHandler().loadTableWithAccessDelegation(TABLE_NS1A_2, "all", Optional.empty()))
+ () ->
+ newHandler()
+ .loadTable(
+ TABLE_NS1A_2,
+ "all",
+ null,
+ EnumSet.of(VENDED_CREDENTIALS),
+ Optional.empty()))
.shouldPassWith(PolarisPrivilege.TABLE_READ_DATA)
.shouldPassWith(PolarisPrivilege.TABLE_WRITE_DATA)
.shouldPassWith(PolarisPrivilege.CATALOG_MANAGE_CONTENT)
@@ -1108,7 +1206,14 @@ public abstract class AbstractIcebergCatalogHandlerAuthzTest extends PolarisAuth
// to receive a read-only credential.
return authzTestsBuilder("loadTableWithAccessDelegation (write)")
.action(
- () -> newHandler().loadTableWithAccessDelegation(TABLE_NS1A_2, "all", Optional.empty()))
+ () ->
+ newHandler()
+ .loadTable(
+ TABLE_NS1A_2,
+ "all",
+ null,
+ EnumSet.of(VENDED_CREDENTIALS),
+ Optional.empty()))
.shouldPassWith(PolarisPrivilege.TABLE_READ_DATA)
.shouldPassWith(PolarisPrivilege.TABLE_WRITE_DATA)
.shouldPassWith(PolarisPrivilege.CATALOG_MANAGE_CONTENT)
@@ -1121,8 +1226,12 @@ public abstract class AbstractIcebergCatalogHandlerAuthzTest extends PolarisAuth
.action(
() ->
newHandler()
- .loadTableWithAccessDelegationIfStale(
- TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", Optional.empty()))
+ .loadTable(
+ TABLE_NS1A_2,
+ "all",
+ IfNoneMatch.fromHeader("W/\"0:0\""),
+ EnumSet.of(VENDED_CREDENTIALS),
+ Optional.empty()))
.shouldPassWith(PolarisPrivilege.TABLE_READ_DATA)
.shouldPassWith(PolarisPrivilege.TABLE_WRITE_DATA)
.shouldPassWith(PolarisPrivilege.CATALOG_MANAGE_CONTENT)
@@ -1138,8 +1247,12 @@ public abstract class AbstractIcebergCatalogHandlerAuthzTest extends PolarisAuth
.action(
() ->
newHandler()
- .loadTableWithAccessDelegationIfStale(
- TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", Optional.empty()))
+ .loadTable(
+ TABLE_NS1A_2,
+ "all",
+ IfNoneMatch.fromHeader("W/\"0:0\""),
+ EnumSet.of(VENDED_CREDENTIALS),
+ Optional.empty()))
.shouldPassWith(PolarisPrivilege.TABLE_READ_DATA)
.shouldPassWith(PolarisPrivilege.TABLE_WRITE_DATA)
.shouldPassWith(PolarisPrivilege.CATALOG_MANAGE_CONTENT)
@@ -2269,7 +2382,15 @@ public abstract class AbstractIcebergCatalogHandlerAuthzTest extends PolarisAuth
// Test that TABLE_MANAGE_STRUCTURE is insufficient for operations that require
// different privilege categories (like read operations)
return authzTestsBuilder("loadTable (TABLE_MANAGE_STRUCTURE insufficient)")
- .action(() -> newHandler().loadTable(TABLE_NS1A_2, "all"))
+ .action(
+ () ->
+ newHandler()
+ .loadTable(
+ TABLE_NS1A_2,
+ "all",
+ null,
+ EnumSet.noneOf(AccessDelegationMode.class),
+ Optional.empty()))
.shouldPassWith(PolarisPrivilege.TABLE_READ_PROPERTIES)
.shouldPassWith(PolarisPrivilege.TABLE_WRITE_PROPERTIES)
.shouldPassWith(PolarisPrivilege.TABLE_READ_DATA)
diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/EntityIdempotencyUpdateTableTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/EntityIdempotencyUpdateTableTest.java
index 040ac43dd0..92c6b2630a 100644
--- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/EntityIdempotencyUpdateTableTest.java
+++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/EntityIdempotencyUpdateTableTest.java
@@ -23,9 +23,11 @@ import static org.assertj.core.api.Assertions.assertThat;
import jakarta.ws.rs.core.Response;
import java.nio.file.Path;
+import java.util.EnumSet;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.UUID;
import org.apache.iceberg.MetadataUpdate;
import org.apache.iceberg.catalog.Namespace;
@@ -40,6 +42,7 @@ import org.apache.polaris.core.admin.model.CreateCatalogRequest;
import org.apache.polaris.core.admin.model.FileStorageConfigInfo;
import org.apache.polaris.core.admin.model.StorageConfigInfo;
import org.apache.polaris.service.TestServices;
+import org.apache.polaris.service.catalog.AccessDelegationMode;
import org.apache.polaris.service.idempotency.IdempotencyRequestContext;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
@@ -147,7 +150,9 @@ public class EntityIdempotencyUpdateTableTest {
return services
.catalogAdapter()
.newHandler(services.securityContext(), CATALOG)
- .loadTable(tableId, "all")
+ .loadTable(
+ tableId, "all", null, EnumSet.noneOf(AccessDelegationMode.class), Optional.empty())
+ .get()
.tableMetadata()
.metadataFileLocation();
}