adam-christian-software commented on code in PR #4816: URL: https://github.com/apache/polaris/pull/4816#discussion_r3454489143
########## spec/polaris-catalog-apis/semantic-models-api.yaml: ########## @@ -0,0 +1,369 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# + +--- +paths: + + /polaris/v1/{prefix}/namespaces/{namespace}/semantic-models: Review Comment: So, the Detailed Design says: > OSI is an evolving specification. Polaris stores the document's declared OSI version (spec_version) and validates the document against the corresponding bundled schema version. > Polaris may support multiple OSI versions simultaneously, allowing users to upgrade Polaris independently of their semantic models. A model written as OSI 0.1.1 remains a 0.1.1 model until explicitly updated by the user. > Documents declaring an unsupported OSI version are rejected with 400 Bad Request. Polaris does not automatically migrate or rewrite semantic models during server upgrades. This follows the same principle as Iceberg format evolution: version upgrades are explicit user actions rather than side effects of infrastructure upgrades. and > Validation > Schema validation: Writes validate against the bundled OSI JSON Schema. A new OsiDocumentValidator produces BadRequestException with JSON-Pointer field paths on failure. The validator is strict: unknown top-level fields cause a 400. Forward compatibility with newer OSI versions is handled by upgrading Polaris's bundled schema as an explicit, coordinated action, not by silently accepting unvalidated content. Vendor-specific or experimental fields belong in custom_extensions, which the schema does allow. > Source table validation: Validate that every dataset.source resolves to a Polaris TABLE_LIKE entity at write time; writes with unresolved sources fail with 400. > Source table column validation: Not supported in v1. So, there won't be any change to the Polaris REST APIs when the OSI Spec changes since it's just passing back-and-forth JSON, but there will be some business logic changes whenever OSI is upgraded in Polaris. ########## runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java: ########## @@ -1534,22 +1436,17 @@ public ConfigResponse getConfig() { Map<String, String> properties = PolarisEntity.of(resolvedReferenceCatalog.getEntity()).getPropertiesAsMap(); + String prefix = prefixParser().catalogNameToPrefix(catalogName()); return ConfigResponse.builder() .withDefaults(properties) // catalog properties are defaults - .withOverrides( - ImmutableMap.of( - "prefix", - prefixParser().catalogNameToPrefix(catalogName()), - // Polaris does not handle custom namespace separators; - // always communicate the default namespace separator to clients. - RESTCatalogProperties.NAMESPACE_SEPARATOR, - NamespaceUtils.DEFAULT_NAMESPACE_SEPARATOR_ENCODED)) + .withOverrides(ImmutableMap.of("prefix", prefix)) .withEndpoints( ImmutableList.<Endpoint>builder() .addAll(DEFAULT_ENDPOINTS) .addAll(VIEW_ENDPOINTS) .addAll(PolarisEndpoints.getSupportedGenericTableEndpoints(realmConfig())) .addAll(PolarisEndpoints.getSupportedPolicyEndpoints(realmConfig())) + .addAll(PolarisEndpoints.getSupportedSemanticModelEndpoints(realmConfig())) Review Comment: Yeah, I wouldn't block the PR on figuring this out. While I agree that advertising the other APIs in the IcebergCatalogHandler is unnecessary coupling and I do think that we should improve this by decoupling it, I do see the pattern here and we should take this as a separate discussion. -- 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]
