jerryshao commented on code in PR #12831:
URL: https://github.com/apache/gravitino/pull/12831#discussion_r4089149729
##########
core/src/main/java/org/apache/gravitino/GravitinoEnv.java:
##########
@@ -1010,7 +1010,13 @@ private void
initSemanticModelDispatcher(SchemaOperationDispatcher schemaOperati
// TODO(#12594): Add Semantic Model ownership and privilege hooks.
SemanticModelOperationDispatcher semanticModelOperationDispatcher =
new SemanticModelOperationDispatcher(
- catalogManager, schemaOperationDispatcher, entityStore,
idGenerator, secretManager);
+ catalogManager,
+ schemaOperationDispatcher,
+ internalTableDispatcher,
+ internalViewDispatcher,
Review Comment:
[Question] Source validation is wired to the *internal* table and view
dispatchers, which by their own javadoc "preserve normalization but skip hooks
and event emission" (GravitinoEnv.java:359-366). Authorization in Gravitino is
applied per top-level REST operation (e.g.
`LoadTableAuthorizationExecutor.java:91`), not inside these nested loads, so
resolving dataset sources during a Semantic Model create is not
privilege-checked against the caller.
That is in tension with the design doc this PR edits, which still states at
`design-docs/gravitino-semantic-model-design.md:277` that "Validation uses the
caller's authorization context so it does not disclose metadata the caller
cannot access". As written, a caller who may create a Semantic Model in one
schema can probe any table in the metalake and distinguish "source does not
exist" from "column 'x' does not exist in source ..." by the error message.
Is this meant to be covered by the `TODO(#12594)` two lines above? If so,
could the doc sentence be softened until the privilege hooks land, so the
documented guarantee matches the code?
Verified by: compared this wiring with the public `tableDispatcher` built at
GravitinoEnv.java:1102-1108 (hook + event wrapped) and read the server-side
authorization executor; confirmed the doc sentence is still present in the
post-PR file.
##########
core/src/main/java/org/apache/gravitino/catalog/ManagedSemanticModelOperations.java:
##########
@@ -80,24 +95,51 @@ public SemanticModel createSemanticModel(
Map<String, String> properties)
throws NoSuchSchemaException, SemanticModelAlreadyExistsException,
IllegalSemanticModelException {
- // TODO: Implement when SemanticModelEntity is available.
- throw new UnsupportedOperationException(
- "createSemanticModel: SemanticModelEntity is not yet implemented");
+ Preconditions.checkArgument(properties != null, "Properties must not be
null");
+
+ writeValidator.accept(ident, definition);
Review Comment:
[Nit] Creating a Semantic Model whose name is already taken runs the full
source validation first and only discovers the conflict at `store.put` (line
119 -> `EntityAlreadyExistsException` at line 121). Because validation resolves
sources through `TableOperationDispatcher.loadTable`, which imports
not-yet-imported external tables under a schema WRITE lock
(TableOperationDispatcher.java:161-181), a create that was always going to fail
can still leave newly imported table entities behind and burn an id from
`idGenerator`. A cheap existence probe before validation, or simply documenting
that the import side effect is acceptable, would avoid the surprise.
Verified by: traced `writeValidator.accept` ->
`SemanticModelValidator.validateSources` -> `tableDispatcher.loadTable` and
read the import branch in `TableOperationDispatcher.loadTable`.
##########
core/src/main/java/org/apache/gravitino/catalog/SemanticModelValidator.java:
##########
@@ -0,0 +1,508 @@
+/*
+ * 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.
+ */
+package org.apache.gravitino.catalog;
+
+import static
org.apache.gravitino.catalog.CapabilityHelpers.applyCaseSensitiveOnName;
+import static org.apache.gravitino.catalog.CapabilityHelpers.getCapability;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import javax.annotation.Nullable;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.connector.capability.Capability;
+import org.apache.gravitino.exceptions.IllegalSemanticModelException;
+import org.apache.gravitino.exceptions.NotFoundException;
+import org.apache.gravitino.rel.Column;
+import org.apache.gravitino.rel.Table;
+import org.apache.gravitino.rel.View;
+import org.apache.gravitino.semantic.AIContext;
+import org.apache.gravitino.semantic.AIContextObject;
+import org.apache.gravitino.semantic.CustomExtension;
+import org.apache.gravitino.semantic.Dataset;
+import org.apache.gravitino.semantic.DialectExpression;
+import org.apache.gravitino.semantic.Expression;
+import org.apache.gravitino.semantic.Field;
+import org.apache.gravitino.semantic.Metric;
+import org.apache.gravitino.semantic.Relationship;
+import org.apache.gravitino.semantic.SemanticModelDefinition;
+
+/**
+ * Validates Semantic Model writes using Gravitino's Java value model and
catalog metadata.
+ *
+ * <p>At implementation time, Apache Ossie commit {@code
88e0011148283302c9a04cd0287e00e0b9d87354},
+ * whose core specification version is {@code 0.2.0.dev0}, did not publish a
reusable Java SDK or
+ * general-purpose Java validator artifact. The Java schema validation in that
upstream tree was
+ * converter-specific. Gravitino therefore implements the applicable
structural and model-local
+ * rules directly in Java. If a future Ossie release publishes a compatible
Java SDK or validator,
+ * Gravitino should evaluate replacing this implementation with that upstream
library.
+ *
+ * <p>{@code validateDefinition} is deterministic and performs no catalog I/O.
{@code
+ * validateSources} resolves Table and logical View metadata and applies
catalog capabilities,
+ * including column case sensitivity. {@code validateForWrite} composes both
phases for create and
+ * definition-replacement writes. SQL expression semantics, transitive View
semantics, and query
+ * engine compatibility are outside this validator's scope.
+ */
+final class SemanticModelValidator {
+
+ private final CatalogManager catalogManager;
+ private final TableDispatcher tableDispatcher;
+ private final ViewDispatcher viewDispatcher;
+
+ SemanticModelValidator(
+ CatalogManager catalogManager,
+ TableDispatcher tableDispatcher,
+ ViewDispatcher viewDispatcher) {
+ Preconditions.checkArgument(catalogManager != null, "Catalog manager must
not be null");
+ Preconditions.checkArgument(tableDispatcher != null, "Table dispatcher
must not be null");
+ Preconditions.checkArgument(viewDispatcher != null, "View dispatcher must
not be null");
+ this.catalogManager = catalogManager;
+ this.tableDispatcher = tableDispatcher;
+ this.viewDispatcher = viewDispatcher;
+ }
+
+ static void validateDefinition(@Nullable SemanticModelDefinition definition)
{
+ if (definition == null) {
+ throw invalid("$", "definition must not be null");
+ }
+
+ validateAIContext(definition.aiContext(), "aiContext");
+
+ Dataset[] datasets = definition.datasets();
+ if (datasets == null || datasets.length == 0) {
+ throw invalid("datasets", "must not be null or empty");
+ }
+
+ Map<String, String> datasetNames = new HashMap<>();
+ for (int index = 0; index < datasets.length; index++) {
+ validateDataset(datasets[index], "datasets[" + index + "]",
datasetNames);
+ }
+
+ validateRelationships(definition.relationships(), datasetNames);
+ validateMetrics(definition.metrics());
+ validateCustomExtensions(definition.customExtensions(),
"customExtensions");
+ }
+
+ void validateSources(String metalake, SemanticModelDefinition definition) {
+ Dataset[] datasets = definition.datasets();
+ Map<String, SourceColumns> columnsByDataset = new HashMap<>();
+
+ for (int datasetIndex = 0; datasetIndex < datasets.length; datasetIndex++)
{
+ Dataset dataset = datasets[datasetIndex];
+ String datasetPath = "datasets[" + datasetIndex + "]";
+ NameIdentifier fullSource =
+ qualifySource(metalake, dataset.source(), datasetPath + ".source");
+ SourceColumns sourceColumns = loadSourceColumns(fullSource, datasetPath
+ ".source");
+ columnsByDataset.put(dataset.name(), sourceColumns);
+
+ validateSourceColumns(dataset.primaryKey(), datasetPath + ".primaryKey",
sourceColumns);
+ String[][] uniqueKeys = dataset.uniqueKeys();
+ if (uniqueKeys != null) {
+ for (int keyIndex = 0; keyIndex < uniqueKeys.length; keyIndex++) {
+ validateSourceColumns(
+ uniqueKeys[keyIndex], datasetPath + ".uniqueKeys[" + keyIndex +
"]", sourceColumns);
+ }
+ }
+ }
+
+ Relationship[] relationships = definition.relationships();
+ if (relationships == null) {
+ return;
+ }
+ for (int relationshipIndex = 0; relationshipIndex < relationships.length;
relationshipIndex++) {
+ Relationship relationship = relationships[relationshipIndex];
+ String path = "relationships[" + relationshipIndex + "]";
+ SourceColumns fromColumns = columnsByDataset.get(relationship.from());
+ SourceColumns toColumns = columnsByDataset.get(relationship.to());
+ if (fromColumns == null || toColumns == null) {
+ throw invalid(
+ path, "relationship endpoints must reference datasets in the same
Semantic Model");
+ }
+ validateSourceColumns(relationship.fromColumns(), path + ".fromColumns",
fromColumns);
+ validateSourceColumns(relationship.toColumns(), path + ".toColumns",
toColumns);
+ }
+ }
+
+ void validateForWrite(
+ NameIdentifier semanticModelIdent, @Nullable SemanticModelDefinition
definition) {
+ if (semanticModelIdent == null || semanticModelIdent.namespace().length()
!= 3) {
+ throw invalid("$", "Semantic Model identifier must use
metalake.catalog.schema.name");
+ }
+ validateDefinition(definition);
+ validateSources(semanticModelIdent.namespace().level(0), definition);
+ }
+
+ private static void validateDataset(
+ @Nullable Dataset dataset, String path, Map<String, String>
datasetNames) {
+ if (dataset == null) {
+ throw invalid(path, "must not be null");
+ }
+
+ String namePath = path + ".name";
+ validateRequiredString(dataset.name(), namePath);
+ validateUniqueName(dataset.name(), namePath, "dataset", datasetNames);
+ validateSource(dataset.source(), path + ".source");
+ validateOptionalColumnNames(dataset.primaryKey(), path + ".primaryKey");
+ validateUniqueKeys(dataset.uniqueKeys(), path + ".uniqueKeys");
+ validateAIContext(dataset.aiContext(), path + ".aiContext");
+ validateFields(dataset.fields(), path + ".fields");
+ validateCustomExtensions(dataset.customExtensions(), path +
".customExtensions");
+ }
+
+ private static void validateSource(@Nullable NameIdentifier source, String
path) {
+ if (source == null) {
+ throw invalid(path, "must not be null");
+ }
+ if (source.namespace().length() != 2) {
+ throw invalid(
+ path, "must contain exactly catalog.schema.name, but was '" +
source.toString() + "'");
+ }
+ }
+
+ private static void validateUniqueKeys(@Nullable String[][] uniqueKeys,
String path) {
+ if (uniqueKeys == null) {
+ return;
+ }
+
+ for (int index = 0; index < uniqueKeys.length; index++) {
+ String keyPath = path + "[" + index + "]";
+ String[] uniqueKey = uniqueKeys[index];
+ if (uniqueKey == null || uniqueKey.length == 0) {
+ throw invalid(keyPath, "must not be null or empty");
+ }
+ validateColumnNames(uniqueKey, keyPath, false);
+ }
+ }
+
+ private static void validateFields(@Nullable Field[] fields, String path) {
+ if (fields == null) {
+ return;
+ }
+
+ Map<String, String> fieldNames = new HashMap<>();
+ for (int index = 0; index < fields.length; index++) {
+ String fieldPath = path + "[" + index + "]";
+ Field field = fields[index];
+ if (field == null) {
+ throw invalid(fieldPath, "must not be null");
+ }
+
+ String namePath = fieldPath + ".name";
+ validateRequiredString(field.name(), namePath);
+ validateUniqueName(field.name(), namePath, "field", fieldNames);
+ validateExpression(field.expression(), fieldPath + ".expression");
+ validateAIContext(field.aiContext(), fieldPath + ".aiContext");
+ validateCustomExtensions(field.customExtensions(), fieldPath +
".customExtensions");
+ }
+ }
+
+ private static void validateRelationships(
+ @Nullable Relationship[] relationships, Map<String, String>
datasetNames) {
+ if (relationships == null) {
+ return;
+ }
+
+ Map<String, String> relationshipNames = new HashMap<>();
+ for (int index = 0; index < relationships.length; index++) {
+ String path = "relationships[" + index + "]";
+ Relationship relationship = relationships[index];
+ if (relationship == null) {
+ throw invalid(path, "must not be null");
+ }
+
+ String namePath = path + ".name";
+ validateRequiredString(relationship.name(), namePath);
+ validateUniqueName(relationship.name(), namePath, "relationship",
relationshipNames);
+ validateEndpoint(relationship.from(), path + ".from", datasetNames);
+ validateEndpoint(relationship.to(), path + ".to", datasetNames);
+
+ String[] fromColumns = relationship.fromColumns();
+ String[] toColumns = relationship.toColumns();
+ validateColumnNames(fromColumns, path + ".fromColumns", false);
+ validateColumnNames(toColumns, path + ".toColumns", false);
+ if (fromColumns.length != toColumns.length) {
+ throw invalid(
+ path + ".toColumns",
+ "must contain "
+ + fromColumns.length
+ + " columns to match "
+ + path
+ + ".fromColumns, but contained "
+ + toColumns.length);
+ }
+ validateAIContext(relationship.aiContext(), path + ".aiContext");
+ validateCustomExtensions(relationship.customExtensions(), path +
".customExtensions");
+ }
+ }
+
+ private static void validateEndpoint(
+ @Nullable String endpoint, String path, Map<String, String>
datasetNames) {
+ validateRequiredString(endpoint, path);
+ if (!datasetNames.containsKey(endpoint)) {
+ throw invalid(
+ path,
+ "unknown dataset '"
+ + endpoint
+ + "'; relationship endpoints must reference datasets in the same
model");
+ }
+ }
+
+ private static void validateMetrics(@Nullable Metric[] metrics) {
+ if (metrics == null) {
+ return;
+ }
+
+ Map<String, String> metricNames = new HashMap<>();
+ for (int index = 0; index < metrics.length; index++) {
+ String path = "metrics[" + index + "]";
+ Metric metric = metrics[index];
+ if (metric == null) {
+ throw invalid(path, "must not be null");
+ }
+
+ String namePath = path + ".name";
+ validateRequiredString(metric.name(), namePath);
+ validateUniqueName(metric.name(), namePath, "metric", metricNames);
+ validateExpression(metric.expression(), path + ".expression");
+ validateAIContext(metric.aiContext(), path + ".aiContext");
+ validateCustomExtensions(metric.customExtensions(), path +
".customExtensions");
+ }
+ }
+
+ private static void validateExpression(@Nullable Expression expression,
String path) {
+ if (expression == null) {
+ throw invalid(path, "must not be null");
+ }
+
+ DialectExpression[] dialectExpressions = expression.dialects();
+ if (dialectExpressions == null || dialectExpressions.length == 0) {
+ throw invalid(path + ".dialects", "must not be null or empty");
+ }
+
+ Map<String, String> dialectPaths = new HashMap<>();
+ for (int index = 0; index < dialectExpressions.length; index++) {
+ String dialectExpressionPath = path + ".dialects[" + index + "]";
+ DialectExpression dialectExpression = dialectExpressions[index];
+ if (dialectExpression == null) {
+ throw invalid(dialectExpressionPath, "must not be null");
+ }
+
+ String dialect = dialectExpression.dialect();
+ String dialectPath = dialectExpressionPath + ".dialect";
+ validateRequiredString(dialect, dialectPath);
+ String firstPath = dialectPaths.putIfAbsent(dialect, dialectPath);
+ if (firstPath != null) {
+ throw invalid(
+ dialectPath, "duplicate dialect '" + dialect + "'; first declared
at " + firstPath);
+ }
+ validateRequiredString(dialectExpression.expression(),
dialectExpressionPath + ".expression");
+ }
+ }
+
+ private static void validateAIContext(@Nullable AIContext aiContext, String
path) {
+ if (aiContext == null) {
+ return;
+ }
+
+ String text = aiContext.text();
+ AIContextObject object = aiContext.object();
+ if ((text == null) == (object == null)) {
+ throw invalid(path, "must contain exactly one string or object value");
+ }
+ if (object == null) {
+ return;
+ }
+
+ validateStringElements(object.synonyms(), path + ".synonyms");
+ validateStringElements(object.examples(), path + ".examples");
+ if (object.additionalProperties() == null) {
+ throw invalid(path + ".additionalProperties", "must not be null");
+ }
+ for (String name : object.additionalProperties().keySet()) {
+ if (name == null) {
+ throw invalid(path + ".additionalProperties", "property name must not
be null");
+ }
+ }
+ }
+
+ private static void validateColumnNames(
+ @Nullable String[] columns, String path, boolean allowEmpty) {
+ if (columns == null || (!allowEmpty && columns.length == 0)) {
+ throw invalid(path, allowEmpty ? "must not be null" : "must not be null
or empty");
+ }
+ for (int index = 0; index < columns.length; index++) {
+ validateRequiredString(columns[index], path + "[" + index + "]");
+ }
+ }
+
+ private static void validateOptionalColumnNames(@Nullable String[] columns,
String path) {
+ if (columns == null) {
+ return;
+ }
+ for (int index = 0; index < columns.length; index++) {
+ validateRequiredString(columns[index], path + "[" + index + "]");
+ }
+ }
+
+ private static void validateStringElements(@Nullable String[] values, String
path) {
+ if (values == null) {
+ return;
+ }
+ for (int index = 0; index < values.length; index++) {
+ if (values[index] == null) {
+ throw invalid(path + "[" + index + "]", "must not be null");
+ }
+ }
+ }
+
+ private static void validateCustomExtensions(
+ @Nullable CustomExtension[] customExtensions, String path) {
+ if (customExtensions == null) {
+ return;
+ }
+ for (int index = 0; index < customExtensions.length; index++) {
+ String extensionPath = path + "[" + index + "]";
+ CustomExtension extension = customExtensions[index];
+ if (extension == null) {
+ throw invalid(extensionPath, "must not be null");
+ }
+ if (extension.vendorName() == null) {
+ throw invalid(extensionPath + ".vendorName", "must not be null");
+ }
+ if (extension.data() == null) {
+ throw invalid(extensionPath + ".data", "must not be null");
+ }
+ }
+ }
+
+ private static void validateRequiredString(@Nullable String value, String
path) {
+ if (value == null || value.isEmpty()) {
+ throw invalid(path, "must not be null or empty");
+ }
+ }
+
+ private static void validateUniqueName(
+ String name, String path, String memberType, Map<String, String> names) {
+ String firstPath = names.putIfAbsent(name, path);
+ if (firstPath != null) {
+ throw invalid(
+ path, "duplicate " + memberType + " name '" + name + "'; first
declared at " + firstPath);
+ }
+ }
+
+ private static NameIdentifier qualifySource(
+ String metalake, NameIdentifier source, String sourcePath) {
+ if (source == null || source.namespace().length() != 2) {
+ throw invalid(sourcePath, "source must use catalog.schema.name");
+ }
+ return NameIdentifier.of(
+ Namespace.of(metalake, source.namespace().level(0),
source.namespace().level(1)),
+ source.name());
+ }
+
+ private SourceColumns loadSourceColumns(NameIdentifier source, String
sourcePath) {
+ Table table;
+ try {
+ table = tableDispatcher.loadTable(source);
+ } catch (RuntimeException tableFailure) {
+ View view;
+ try {
+ view = viewDispatcher.loadView(source);
+ } catch (RuntimeException viewFailure) {
+ if (tableFailure instanceof NotFoundException && viewFailure
instanceof NotFoundException) {
+ IllegalSemanticModelException failure =
+ new IllegalSemanticModelException(
+ viewFailure,
+ "%s: source %s does not exist as a Table or logical View",
+ sourcePath,
+ source);
+ failure.addSuppressed(tableFailure);
+ throw failure;
+ }
+
+ // Prefer an operational failure over a missing source, retaining both
lookup failures.
+ RuntimeException failure =
+ tableFailure instanceof NotFoundException ? viewFailure :
tableFailure;
Review Comment:
[Important] For every relational catalog that has no view support, a simple
typo in a dataset source now surfaces as `UnsupportedOperationException:
Catalog does not support view operations` instead of the documented
`IllegalSemanticModelException`.
`CatalogManager.CatalogWrapper.doWithViewOps` (CatalogManager.java:318-325)
throws `UnsupportedOperationException("Catalog does not support view
operations")` whenever `asViews()` is null (CatalogManager.java:487-489), and
`JdbcCatalogOperations` — the base for MySQL, PostgreSQL and the
MySQL-protocol-compatible catalogs — declares only `CatalogOperations,
SupportsSchemas, TableCatalog`
(catalogs/catalog-jdbc-common/.../JdbcCatalogOperations.java:85).
`OperationDispatcher.doWithCatalog` rethrows any `RuntimeException` unchanged
(OperationDispatcher.java:111-119), so `viewDispatcher.loadView` on such a
catalog *always* fails this way, never with a `NotFoundException`.
So on a MySQL/PostgreSQL source: `tableFailure` is `NoSuchTableException` (a
`NotFoundException`), `viewFailure` is that `UnsupportedOperationException`,
the ternary here selects `viewFailure`, and the not-found branch at line 434 is
unreachable. The same happens when a source points into a non-relational
catalog, where the table lookup is unsupported too. Only the model's own
catalog is checked for `Catalog.Type.RELATIONAL`
(SemanticModelOperationDispatcher.java:127-136); sources are not. Once this
path is wired to REST, a user typo becomes an internal error rather than a
400-class validation failure.
Deciding this from the exception type cannot work, because "this catalog has
no views at all" and the real operational conflict Copilot flagged are both
`UnsupportedOperationException`. Suggest establishing the source catalog's view
support up front instead — the validator already loads the source catalog in
`sourceColumns` via `getCapability(source, catalogManager)` — and only
attempting the view fallback when views exist; otherwise treat a not-found
table as the missing source.
Verified by: read `CatalogManager.doWithViewOps`/`asViews`,
`OperationDispatcher.doWithCatalog`, `JdbcCatalogOperations`'s interface list
and `NoSuchTableException extends NotFoundException`; confirmed with `grep -rn
"class .*implements.*ViewCatalog"` that only Hive (plus Iceberg/Paimon catalog
ops) provide view operations.
##########
core/src/main/java/org/apache/gravitino/catalog/SemanticModelValidator.java:
##########
@@ -0,0 +1,508 @@
+/*
+ * 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.
+ */
+package org.apache.gravitino.catalog;
+
+import static
org.apache.gravitino.catalog.CapabilityHelpers.applyCaseSensitiveOnName;
+import static org.apache.gravitino.catalog.CapabilityHelpers.getCapability;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import javax.annotation.Nullable;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.connector.capability.Capability;
+import org.apache.gravitino.exceptions.IllegalSemanticModelException;
+import org.apache.gravitino.exceptions.NotFoundException;
+import org.apache.gravitino.rel.Column;
+import org.apache.gravitino.rel.Table;
+import org.apache.gravitino.rel.View;
+import org.apache.gravitino.semantic.AIContext;
+import org.apache.gravitino.semantic.AIContextObject;
+import org.apache.gravitino.semantic.CustomExtension;
+import org.apache.gravitino.semantic.Dataset;
+import org.apache.gravitino.semantic.DialectExpression;
+import org.apache.gravitino.semantic.Expression;
+import org.apache.gravitino.semantic.Field;
+import org.apache.gravitino.semantic.Metric;
+import org.apache.gravitino.semantic.Relationship;
+import org.apache.gravitino.semantic.SemanticModelDefinition;
+
+/**
+ * Validates Semantic Model writes using Gravitino's Java value model and
catalog metadata.
+ *
+ * <p>At implementation time, Apache Ossie commit {@code
88e0011148283302c9a04cd0287e00e0b9d87354},
+ * whose core specification version is {@code 0.2.0.dev0}, did not publish a
reusable Java SDK or
+ * general-purpose Java validator artifact. The Java schema validation in that
upstream tree was
+ * converter-specific. Gravitino therefore implements the applicable
structural and model-local
+ * rules directly in Java. If a future Ossie release publishes a compatible
Java SDK or validator,
+ * Gravitino should evaluate replacing this implementation with that upstream
library.
+ *
+ * <p>{@code validateDefinition} is deterministic and performs no catalog I/O.
{@code
+ * validateSources} resolves Table and logical View metadata and applies
catalog capabilities,
+ * including column case sensitivity. {@code validateForWrite} composes both
phases for create and
+ * definition-replacement writes. SQL expression semantics, transitive View
semantics, and query
+ * engine compatibility are outside this validator's scope.
+ */
+final class SemanticModelValidator {
+
+ private final CatalogManager catalogManager;
+ private final TableDispatcher tableDispatcher;
+ private final ViewDispatcher viewDispatcher;
+
+ SemanticModelValidator(
+ CatalogManager catalogManager,
+ TableDispatcher tableDispatcher,
+ ViewDispatcher viewDispatcher) {
+ Preconditions.checkArgument(catalogManager != null, "Catalog manager must
not be null");
+ Preconditions.checkArgument(tableDispatcher != null, "Table dispatcher
must not be null");
+ Preconditions.checkArgument(viewDispatcher != null, "View dispatcher must
not be null");
+ this.catalogManager = catalogManager;
+ this.tableDispatcher = tableDispatcher;
+ this.viewDispatcher = viewDispatcher;
+ }
+
+ static void validateDefinition(@Nullable SemanticModelDefinition definition)
{
+ if (definition == null) {
+ throw invalid("$", "definition must not be null");
+ }
+
+ validateAIContext(definition.aiContext(), "aiContext");
+
+ Dataset[] datasets = definition.datasets();
+ if (datasets == null || datasets.length == 0) {
+ throw invalid("datasets", "must not be null or empty");
+ }
+
+ Map<String, String> datasetNames = new HashMap<>();
+ for (int index = 0; index < datasets.length; index++) {
+ validateDataset(datasets[index], "datasets[" + index + "]",
datasetNames);
+ }
+
+ validateRelationships(definition.relationships(), datasetNames);
+ validateMetrics(definition.metrics());
+ validateCustomExtensions(definition.customExtensions(),
"customExtensions");
+ }
+
+ void validateSources(String metalake, SemanticModelDefinition definition) {
+ Dataset[] datasets = definition.datasets();
+ Map<String, SourceColumns> columnsByDataset = new HashMap<>();
+
+ for (int datasetIndex = 0; datasetIndex < datasets.length; datasetIndex++)
{
+ Dataset dataset = datasets[datasetIndex];
+ String datasetPath = "datasets[" + datasetIndex + "]";
+ NameIdentifier fullSource =
+ qualifySource(metalake, dataset.source(), datasetPath + ".source");
+ SourceColumns sourceColumns = loadSourceColumns(fullSource, datasetPath
+ ".source");
Review Comment:
[Nit] `loadSourceColumns` is called once per dataset with no memoization, so
a model whose datasets share a source (a common shape: several datasets over
the same wide fact table) issues one dispatcher load per dataset. Each of those
is not cheap: `TableOperationDispatcher.loadTable` takes a tree READ lock and,
for a table Gravitino has not imported yet, takes a schema WRITE lock and
imports it (TableOperationDispatcher.java:161-181). Caching `SourceColumns` in
a `Map<NameIdentifier, SourceColumns>` keyed by the qualified source would
collapse those to one load per distinct source.
Verified by: read `TableOperationDispatcher.loadTable` and the per-dataset
loop here.
##########
core/src/main/java/org/apache/gravitino/catalog/SemanticModelValidator.java:
##########
@@ -0,0 +1,508 @@
+/*
+ * 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.
+ */
+package org.apache.gravitino.catalog;
+
+import static
org.apache.gravitino.catalog.CapabilityHelpers.applyCaseSensitiveOnName;
+import static org.apache.gravitino.catalog.CapabilityHelpers.getCapability;
+
+import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import javax.annotation.Nullable;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.connector.capability.Capability;
+import org.apache.gravitino.exceptions.IllegalSemanticModelException;
+import org.apache.gravitino.exceptions.NotFoundException;
+import org.apache.gravitino.rel.Column;
+import org.apache.gravitino.rel.Table;
+import org.apache.gravitino.rel.View;
+import org.apache.gravitino.semantic.AIContext;
+import org.apache.gravitino.semantic.AIContextObject;
+import org.apache.gravitino.semantic.CustomExtension;
+import org.apache.gravitino.semantic.Dataset;
+import org.apache.gravitino.semantic.DialectExpression;
+import org.apache.gravitino.semantic.Expression;
+import org.apache.gravitino.semantic.Field;
+import org.apache.gravitino.semantic.Metric;
+import org.apache.gravitino.semantic.Relationship;
+import org.apache.gravitino.semantic.SemanticModelDefinition;
+
+/**
+ * Validates Semantic Model writes using Gravitino's Java value model and
catalog metadata.
+ *
+ * <p>At implementation time, Apache Ossie commit {@code
88e0011148283302c9a04cd0287e00e0b9d87354},
+ * whose core specification version is {@code 0.2.0.dev0}, did not publish a
reusable Java SDK or
+ * general-purpose Java validator artifact. The Java schema validation in that
upstream tree was
+ * converter-specific. Gravitino therefore implements the applicable
structural and model-local
+ * rules directly in Java. If a future Ossie release publishes a compatible
Java SDK or validator,
+ * Gravitino should evaluate replacing this implementation with that upstream
library.
+ *
+ * <p>{@code validateDefinition} is deterministic and performs no catalog I/O.
{@code
+ * validateSources} resolves Table and logical View metadata and applies
catalog capabilities,
+ * including column case sensitivity. {@code validateForWrite} composes both
phases for create and
+ * definition-replacement writes. SQL expression semantics, transitive View
semantics, and query
+ * engine compatibility are outside this validator's scope.
+ */
+final class SemanticModelValidator {
+
+ private final CatalogManager catalogManager;
+ private final TableDispatcher tableDispatcher;
+ private final ViewDispatcher viewDispatcher;
+
+ SemanticModelValidator(
+ CatalogManager catalogManager,
+ TableDispatcher tableDispatcher,
+ ViewDispatcher viewDispatcher) {
+ Preconditions.checkArgument(catalogManager != null, "Catalog manager must
not be null");
+ Preconditions.checkArgument(tableDispatcher != null, "Table dispatcher
must not be null");
+ Preconditions.checkArgument(viewDispatcher != null, "View dispatcher must
not be null");
+ this.catalogManager = catalogManager;
+ this.tableDispatcher = tableDispatcher;
+ this.viewDispatcher = viewDispatcher;
+ }
+
+ static void validateDefinition(@Nullable SemanticModelDefinition definition)
{
+ if (definition == null) {
+ throw invalid("$", "definition must not be null");
+ }
+
+ validateAIContext(definition.aiContext(), "aiContext");
+
+ Dataset[] datasets = definition.datasets();
+ if (datasets == null || datasets.length == 0) {
+ throw invalid("datasets", "must not be null or empty");
+ }
+
+ Map<String, String> datasetNames = new HashMap<>();
+ for (int index = 0; index < datasets.length; index++) {
+ validateDataset(datasets[index], "datasets[" + index + "]",
datasetNames);
+ }
+
+ validateRelationships(definition.relationships(), datasetNames);
+ validateMetrics(definition.metrics());
+ validateCustomExtensions(definition.customExtensions(),
"customExtensions");
+ }
+
+ void validateSources(String metalake, SemanticModelDefinition definition) {
+ Dataset[] datasets = definition.datasets();
+ Map<String, SourceColumns> columnsByDataset = new HashMap<>();
+
+ for (int datasetIndex = 0; datasetIndex < datasets.length; datasetIndex++)
{
+ Dataset dataset = datasets[datasetIndex];
+ String datasetPath = "datasets[" + datasetIndex + "]";
+ NameIdentifier fullSource =
+ qualifySource(metalake, dataset.source(), datasetPath + ".source");
+ SourceColumns sourceColumns = loadSourceColumns(fullSource, datasetPath
+ ".source");
+ columnsByDataset.put(dataset.name(), sourceColumns);
+
+ validateSourceColumns(dataset.primaryKey(), datasetPath + ".primaryKey",
sourceColumns);
+ String[][] uniqueKeys = dataset.uniqueKeys();
+ if (uniqueKeys != null) {
+ for (int keyIndex = 0; keyIndex < uniqueKeys.length; keyIndex++) {
+ validateSourceColumns(
+ uniqueKeys[keyIndex], datasetPath + ".uniqueKeys[" + keyIndex +
"]", sourceColumns);
+ }
+ }
+ }
+
+ Relationship[] relationships = definition.relationships();
+ if (relationships == null) {
+ return;
+ }
+ for (int relationshipIndex = 0; relationshipIndex < relationships.length;
relationshipIndex++) {
+ Relationship relationship = relationships[relationshipIndex];
+ String path = "relationships[" + relationshipIndex + "]";
+ SourceColumns fromColumns = columnsByDataset.get(relationship.from());
+ SourceColumns toColumns = columnsByDataset.get(relationship.to());
+ if (fromColumns == null || toColumns == null) {
+ throw invalid(
+ path, "relationship endpoints must reference datasets in the same
Semantic Model");
+ }
+ validateSourceColumns(relationship.fromColumns(), path + ".fromColumns",
fromColumns);
+ validateSourceColumns(relationship.toColumns(), path + ".toColumns",
toColumns);
+ }
+ }
+
+ void validateForWrite(
+ NameIdentifier semanticModelIdent, @Nullable SemanticModelDefinition
definition) {
+ if (semanticModelIdent == null || semanticModelIdent.namespace().length()
!= 3) {
+ throw invalid("$", "Semantic Model identifier must use
metalake.catalog.schema.name");
+ }
+ validateDefinition(definition);
+ validateSources(semanticModelIdent.namespace().level(0), definition);
+ }
+
+ private static void validateDataset(
+ @Nullable Dataset dataset, String path, Map<String, String>
datasetNames) {
+ if (dataset == null) {
+ throw invalid(path, "must not be null");
+ }
+
+ String namePath = path + ".name";
+ validateRequiredString(dataset.name(), namePath);
+ validateUniqueName(dataset.name(), namePath, "dataset", datasetNames);
+ validateSource(dataset.source(), path + ".source");
+ validateOptionalColumnNames(dataset.primaryKey(), path + ".primaryKey");
+ validateUniqueKeys(dataset.uniqueKeys(), path + ".uniqueKeys");
+ validateAIContext(dataset.aiContext(), path + ".aiContext");
+ validateFields(dataset.fields(), path + ".fields");
+ validateCustomExtensions(dataset.customExtensions(), path +
".customExtensions");
+ }
+
+ private static void validateSource(@Nullable NameIdentifier source, String
path) {
+ if (source == null) {
+ throw invalid(path, "must not be null");
+ }
+ if (source.namespace().length() != 2) {
+ throw invalid(
+ path, "must contain exactly catalog.schema.name, but was '" +
source.toString() + "'");
+ }
+ }
+
+ private static void validateUniqueKeys(@Nullable String[][] uniqueKeys,
String path) {
+ if (uniqueKeys == null) {
+ return;
+ }
+
+ for (int index = 0; index < uniqueKeys.length; index++) {
+ String keyPath = path + "[" + index + "]";
+ String[] uniqueKey = uniqueKeys[index];
+ if (uniqueKey == null || uniqueKey.length == 0) {
+ throw invalid(keyPath, "must not be null or empty");
+ }
+ validateColumnNames(uniqueKey, keyPath, false);
+ }
+ }
+
+ private static void validateFields(@Nullable Field[] fields, String path) {
+ if (fields == null) {
+ return;
+ }
+
+ Map<String, String> fieldNames = new HashMap<>();
+ for (int index = 0; index < fields.length; index++) {
+ String fieldPath = path + "[" + index + "]";
+ Field field = fields[index];
+ if (field == null) {
+ throw invalid(fieldPath, "must not be null");
+ }
+
+ String namePath = fieldPath + ".name";
+ validateRequiredString(field.name(), namePath);
+ validateUniqueName(field.name(), namePath, "field", fieldNames);
+ validateExpression(field.expression(), fieldPath + ".expression");
+ validateAIContext(field.aiContext(), fieldPath + ".aiContext");
+ validateCustomExtensions(field.customExtensions(), fieldPath +
".customExtensions");
+ }
+ }
+
+ private static void validateRelationships(
+ @Nullable Relationship[] relationships, Map<String, String>
datasetNames) {
+ if (relationships == null) {
+ return;
+ }
+
+ Map<String, String> relationshipNames = new HashMap<>();
+ for (int index = 0; index < relationships.length; index++) {
+ String path = "relationships[" + index + "]";
+ Relationship relationship = relationships[index];
+ if (relationship == null) {
+ throw invalid(path, "must not be null");
+ }
+
+ String namePath = path + ".name";
+ validateRequiredString(relationship.name(), namePath);
+ validateUniqueName(relationship.name(), namePath, "relationship",
relationshipNames);
+ validateEndpoint(relationship.from(), path + ".from", datasetNames);
+ validateEndpoint(relationship.to(), path + ".to", datasetNames);
+
+ String[] fromColumns = relationship.fromColumns();
+ String[] toColumns = relationship.toColumns();
+ validateColumnNames(fromColumns, path + ".fromColumns", false);
+ validateColumnNames(toColumns, path + ".toColumns", false);
+ if (fromColumns.length != toColumns.length) {
+ throw invalid(
+ path + ".toColumns",
+ "must contain "
+ + fromColumns.length
+ + " columns to match "
+ + path
+ + ".fromColumns, but contained "
+ + toColumns.length);
+ }
+ validateAIContext(relationship.aiContext(), path + ".aiContext");
+ validateCustomExtensions(relationship.customExtensions(), path +
".customExtensions");
+ }
+ }
+
+ private static void validateEndpoint(
+ @Nullable String endpoint, String path, Map<String, String>
datasetNames) {
+ validateRequiredString(endpoint, path);
+ if (!datasetNames.containsKey(endpoint)) {
+ throw invalid(
+ path,
+ "unknown dataset '"
+ + endpoint
+ + "'; relationship endpoints must reference datasets in the same
model");
+ }
+ }
+
+ private static void validateMetrics(@Nullable Metric[] metrics) {
+ if (metrics == null) {
+ return;
+ }
+
+ Map<String, String> metricNames = new HashMap<>();
+ for (int index = 0; index < metrics.length; index++) {
+ String path = "metrics[" + index + "]";
+ Metric metric = metrics[index];
+ if (metric == null) {
+ throw invalid(path, "must not be null");
+ }
+
+ String namePath = path + ".name";
+ validateRequiredString(metric.name(), namePath);
+ validateUniqueName(metric.name(), namePath, "metric", metricNames);
+ validateExpression(metric.expression(), path + ".expression");
+ validateAIContext(metric.aiContext(), path + ".aiContext");
+ validateCustomExtensions(metric.customExtensions(), path +
".customExtensions");
+ }
+ }
+
+ private static void validateExpression(@Nullable Expression expression,
String path) {
+ if (expression == null) {
+ throw invalid(path, "must not be null");
+ }
+
+ DialectExpression[] dialectExpressions = expression.dialects();
+ if (dialectExpressions == null || dialectExpressions.length == 0) {
+ throw invalid(path + ".dialects", "must not be null or empty");
+ }
+
+ Map<String, String> dialectPaths = new HashMap<>();
+ for (int index = 0; index < dialectExpressions.length; index++) {
+ String dialectExpressionPath = path + ".dialects[" + index + "]";
+ DialectExpression dialectExpression = dialectExpressions[index];
+ if (dialectExpression == null) {
+ throw invalid(dialectExpressionPath, "must not be null");
+ }
+
+ String dialect = dialectExpression.dialect();
+ String dialectPath = dialectExpressionPath + ".dialect";
+ validateRequiredString(dialect, dialectPath);
+ String firstPath = dialectPaths.putIfAbsent(dialect, dialectPath);
+ if (firstPath != null) {
+ throw invalid(
+ dialectPath, "duplicate dialect '" + dialect + "'; first declared
at " + firstPath);
+ }
+ validateRequiredString(dialectExpression.expression(),
dialectExpressionPath + ".expression");
+ }
+ }
+
+ private static void validateAIContext(@Nullable AIContext aiContext, String
path) {
+ if (aiContext == null) {
+ return;
+ }
+
+ String text = aiContext.text();
+ AIContextObject object = aiContext.object();
+ if ((text == null) == (object == null)) {
+ throw invalid(path, "must contain exactly one string or object value");
+ }
+ if (object == null) {
+ return;
+ }
+
+ validateStringElements(object.synonyms(), path + ".synonyms");
+ validateStringElements(object.examples(), path + ".examples");
+ if (object.additionalProperties() == null) {
+ throw invalid(path + ".additionalProperties", "must not be null");
+ }
+ for (String name : object.additionalProperties().keySet()) {
+ if (name == null) {
+ throw invalid(path + ".additionalProperties", "property name must not
be null");
+ }
+ }
+ }
+
+ private static void validateColumnNames(
+ @Nullable String[] columns, String path, boolean allowEmpty) {
+ if (columns == null || (!allowEmpty && columns.length == 0)) {
+ throw invalid(path, allowEmpty ? "must not be null" : "must not be null
or empty");
+ }
+ for (int index = 0; index < columns.length; index++) {
+ validateRequiredString(columns[index], path + "[" + index + "]");
+ }
+ }
+
+ private static void validateOptionalColumnNames(@Nullable String[] columns,
String path) {
+ if (columns == null) {
+ return;
+ }
+ for (int index = 0; index < columns.length; index++) {
+ validateRequiredString(columns[index], path + "[" + index + "]");
+ }
+ }
+
+ private static void validateStringElements(@Nullable String[] values, String
path) {
+ if (values == null) {
+ return;
+ }
+ for (int index = 0; index < values.length; index++) {
+ if (values[index] == null) {
+ throw invalid(path + "[" + index + "]", "must not be null");
+ }
+ }
+ }
+
+ private static void validateCustomExtensions(
+ @Nullable CustomExtension[] customExtensions, String path) {
+ if (customExtensions == null) {
+ return;
+ }
+ for (int index = 0; index < customExtensions.length; index++) {
+ String extensionPath = path + "[" + index + "]";
+ CustomExtension extension = customExtensions[index];
+ if (extension == null) {
+ throw invalid(extensionPath, "must not be null");
+ }
+ if (extension.vendorName() == null) {
+ throw invalid(extensionPath + ".vendorName", "must not be null");
Review Comment:
[Nit] `vendorName` and `data` are only null-checked here, while every other
required string in this validator goes through `validateRequiredString` (line
400-404), which also rejects `""`. So `CustomExtension` with `vendorName = ""`
passes validation and is persisted, whereas an empty dataset, field, metric,
relationship or dialect name is rejected. If that asymmetry is not deliberate,
`validateRequiredString(extension.vendorName(), extensionPath + ".vendorName")`
would make it consistent.
Verified by: compared with the `validateRequiredString` call sites at lines
164, 213, 236, 286 and 314-320.
--
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]
