dimas-b commented on code in PR #4606:
URL: https://github.com/apache/polaris/pull/4606#discussion_r3357522774
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -509,12 +511,105 @@ public LoadTableResponse createTableDirect(
throw new IllegalStateException("Cannot wrap catalog that does not produce
BaseTable");
}
+ /**
+ * Table/view properties through which a caller can specify a storage
location, in addition to the
+ * top-level {@code location} field. These redirect where data/metadata
files are written, so they
+ * are gated by the same configuration as an explicit location.
+ */
+ private static final List<String> CLIENT_LOCATION_PROPERTY_KEYS =
+ List.of(
+ IcebergTableLikeEntity.USER_SPECIFIED_WRITE_DATA_LOCATION_KEY,
+ IcebergTableLikeEntity.USER_SPECIFIED_WRITE_METADATA_LOCATION_KEY);
+
+ /**
+ * Rejects a caller-supplied table or view location unless the catalog is
configured to allow it.
+ * By default Polaris generates the managed location for new tables and
views; honoring a
+ * caller-specified location is opt-in (see {@link
+ * FeatureConfiguration#ALLOW_CLIENT_SPECIFIED_TABLE_LOCATION}). This covers
both the top-level
+ * {@code location} field and the {@code write.data.path} / {@code
write.metadata.path}
+ * properties.
+ */
+ private void rejectClientSpecifiedLocationIfDisallowed(
+ String requestLocation, Map<String, String> requestProperties) {
+ enforceClientSpecifiedLocationAllowed(
+ clientSpecifiedLocationSource(requestLocation, requestProperties));
+ }
+
+ /**
+ * Rejects an update that changes a table/view location (via {@code
SetLocation} or the {@code
+ * write.data.path} / {@code write.metadata.path} properties) unless the
catalog is configured to
+ * allow caller-specified locations.
+ */
+ private void rejectClientSpecifiedLocationIfDisallowed(UpdateTableRequest
request) {
+
enforceClientSpecifiedLocationAllowed(clientSpecifiedLocationSource(request));
+ }
+
+ private void enforceClientSpecifiedLocationAllowed(String specifiedBy) {
+ // Federated catalogs delegate location handling to the remote catalog, so
this constraint
+ // only applies to Polaris-managed catalogs.
+ if (specifiedBy == null || isFederated) {
+ return;
+ }
+ if (!realmConfig()
+ .getConfig(
+ FeatureConfiguration.ALLOW_CLIENT_SPECIFIED_TABLE_LOCATION,
+ getResolvedCatalogEntity())) {
+ throw new BadRequestException(
+ "Specifying a table or view location (%s) is not allowed; set %s to
true to allow it.",
Review Comment:
Re: `set %s to true to allow it` - if we disallow it for a reason, we should
_not_ suggest an "easy button" for by-passing this check. Instead users should
read the docs and make a mindful decision, I think.
--
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]
