flyrain commented on code in PR #3997:
URL: https://github.com/apache/polaris/pull/3997#discussion_r2998411774
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -740,6 +738,71 @@ public Optional<LoadTableResponse>
loadTableWithAccessDelegationIfStale(
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
properties.
+ */
+ public ImmutableLoadCredentialsResponse loadCredentialsFromEntityProperties(
+ TableIdentifier tableIdentifier, Optional<String>
refreshCredentialsEndpoint) {
+
+ Set<PolarisStorageActions> actionsRequested =
+ authorizeLoadTable(tableIdentifier, EnumSet.of(VENDED_CREDENTIALS));
+
+ IcebergTableLikeEntity entity = getTableEntity(tableIdentifier);
+ if (entity == null) {
+ LOGGER
+ .atDebug()
+ .addKeyValue("tableIdentifier", tableIdentifier)
+ .log(
+ "Entity not found in metastore (external catalog?), "
+ + "falling back to full loadTable path");
+ return fallbackToFullLoadTable(tableIdentifier,
refreshCredentialsEndpoint);
+ }
+
+ Map<String, String> internalProperties =
entity.getInternalPropertiesAsMap();
+ String baseLocation =
internalProperties.get(IcebergTableLikeEntity.LOCATION);
+
+ if (baseLocation == null) {
+ LOGGER
+ .atDebug()
+ .addKeyValue("tableIdentifier", tableIdentifier)
+ .log(
+ "Entity missing location in internal properties, requires
backfill "
+ + "as it was likely not updated with stored property
changes. "
+ + "Falling back to full loadTable path");
+ return fallbackToFullLoadTable(tableIdentifier,
refreshCredentialsEndpoint);
+ }
+
+ Set<String> tableLocations =
+ StorageUtil.getLocationsUsedByTable(baseLocation, internalProperties);
Review Comment:
We need a strong guarantee at table property write time that all relevant
locations are persisted, including base location, `write.metadata.path` and
`write.data.path`. I assume this is already handled, but any future changes
could silently break this guarantee. It would be helpful to add a test case to
validate this behavior at write time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
