mchades commented on code in PR #12831:
URL: https://github.com/apache/gravitino/pull/12831#discussion_r4091376197


##########
core/src/main/java/org/apache/gravitino/catalog/SemanticModelValidator.java:
##########
@@ -0,0 +1,350 @@
+/*
+ * 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 java.util.HashMap;
+import java.util.Map;
+import javax.annotation.Nullable;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.exceptions.IllegalSemanticModelException;
+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.
+ *
+ * <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>Validation is deterministic and performs no catalog I/O. Catalog-backed 
source validation,
+ * including existence, columns, and authorization, must be completed by the 
caller before this
+ * validator is invoked. SQL expression semantics, transitive View semantics, 
and query engine
+ * compatibility are outside this validator's scope.
+ */
+final class SemanticModelValidator {
+
+  private SemanticModelValidator() {}
+
+  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");
+  }
+
+  // TODO(#12594): Validate source existence, columns, and authorization in 
the caller before
+  // invoking this definition-only validator.
+  static 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);
+  }
+
+  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");
+    }

Review Comment:
   Trimmed the validator to the model-level rules the value objects cannot 
enforce: duplicate dataset, field, relationship, and metric names; relationship 
endpoint resolution; source identifier shape; and the null-definition guard. 
Removed the duplicate structural checks and their mock-only tests, and updated 
the class Javadoc to document the boundary.



##########
core/src/main/java/org/apache/gravitino/catalog/SemanticModelValidator.java:
##########
@@ -0,0 +1,350 @@
+/*
+ * 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 java.util.HashMap;
+import java.util.Map;
+import javax.annotation.Nullable;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.exceptions.IllegalSemanticModelException;
+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.
+ *
+ * <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>Validation is deterministic and performs no catalog I/O. Catalog-backed 
source validation,
+ * including existence, columns, and authorization, must be completed by the 
caller before this
+ * validator is invoked. SQL expression semantics, transitive View semantics, 
and query engine
+ * compatibility are outside this validator's scope.
+ */
+final class SemanticModelValidator {
+
+  private SemanticModelValidator() {}
+
+  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");
+  }
+
+  // TODO(#12594): Validate source existence, columns, and authorization in 
the caller before
+  // invoking this definition-only validator.
+  static 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);
+  }

Review Comment:
   Removed `validateForWrite` and its direct-only identifier test. The 
dispatcher now invokes `validateDefinition`; persistence continues to guard 
identifiers through `NameIdentifierUtil.checkSemanticModel`.



##########
core/build.gradle.kts:
##########
@@ -43,6 +43,7 @@ dependencies {
   implementation(libs.concurrent.trees)
   implementation(libs.guava)
   implementation(libs.h2db)
+  implementation(libs.jackson.databind)

Review Comment:
   Removed `implementation(libs.jackson.databind)`. It was a leftover from the 
pre-rebase branch, and this PR does not introduce a new Jackson dependency 
requirement.



##########
core/src/main/java/org/apache/gravitino/catalog/SemanticModelValidator.java:
##########
@@ -0,0 +1,350 @@
+/*
+ * 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 java.util.HashMap;
+import java.util.Map;
+import javax.annotation.Nullable;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.exceptions.IllegalSemanticModelException;
+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.
+ *
+ * <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>Validation is deterministic and performs no catalog I/O. Catalog-backed 
source validation,
+ * including existence, columns, and authorization, must be completed by the 
caller before this
+ * validator is invoked. SQL expression semantics, transitive View semantics, 
and query engine
+ * compatibility are outside this validator's scope.
+ */
+final class SemanticModelValidator {
+
+  private SemanticModelValidator() {}
+
+  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");
+  }
+
+  // TODO(#12594): Validate source existence, columns, and authorization in 
the caller before
+  // invoking this definition-only validator.
+  static 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);
+  }
+
+  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 + "]");
+    }
+  }

Review Comment:
   Removed `validateColumnNames` and the `allowEmpty` parameter together with 
the duplicate value-level validation.



##########
core/src/test/java/org/apache/gravitino/catalog/TestManagedSemanticModelOperationsJDBC.java:
##########
@@ -0,0 +1,124 @@
+/*
+ * 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.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotSame;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.IOException;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.apache.commons.lang3.reflect.FieldUtils;
+import org.apache.gravitino.Config;
+import org.apache.gravitino.Configs;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.GravitinoEnv;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.cache.NoOpsCache;
+import org.apache.gravitino.semantic.CustomExtension;
+import org.apache.gravitino.semantic.Dataset;
+import org.apache.gravitino.semantic.Metric;
+import org.apache.gravitino.semantic.Relationship;
+import org.apache.gravitino.semantic.SemanticModel;
+import org.apache.gravitino.semantic.SemanticModelDefinition;
+import org.apache.gravitino.storage.RandomIdGenerator;
+import org.apache.gravitino.storage.relational.RelationalEntityStore;
+import org.apache.gravitino.storage.relational.TestJDBCBackend;
+import org.apache.gravitino.utils.NamespaceUtil;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.TestTemplate;
+
+/** Verifies managed create and load through a real relational persistence 
backend. */
+public class TestManagedSemanticModelOperationsJDBC extends TestJDBCBackend {
+
+  private final AtomicInteger writeValidationCount = new AtomicInteger();
+
+  private Config previousConfig;
+  private NameIdentifier modelIdent;
+  private ManagedSemanticModelOperations operations;
+
+  @BeforeAll
+  public void captureEnvironmentConfig() {
+    previousConfig = GravitinoEnv.getInstance().config();
+  }
+
+  @BeforeEach
+  public void prepareManagedOperations() throws IOException, 
IllegalAccessException {
+    writeValidationCount.set(0);
+    String metalake = "managed_semantic_model_metalake";
+    String catalog = "managed_semantic_model_catalog";
+    String schema = "managed_semantic_model_schema";
+    createAndInsertMakeLake(metalake);
+    createAndInsertCatalog(metalake, catalog);
+    createAndInsertSchema(metalake, catalog, schema);
+
+    Namespace namespace = NamespaceUtil.ofSemanticModel(metalake, catalog, 
schema);
+    modelIdent = NameIdentifier.of(namespace, "sales_model");
+
+    Config config = new Config(false) {};
+    config.set(Configs.CACHE_ENABLED, false);
+    RelationalEntityStore store = new RelationalEntityStore();
+    FieldUtils.writeField(store, "backend", backend, true);
+    FieldUtils.writeField(store, "cache", new NoOpsCache(config), true);
+    operations =
+        new ManagedSemanticModelOperations(
+            store,
+            RandomIdGenerator.INSTANCE,
+            (ident, definition) -> writeValidationCount.incrementAndGet());
+  }
+
+  @AfterEach
+  public void restoreEnvironmentConfig() throws IllegalAccessException {
+    FieldUtils.writeField(GravitinoEnv.getInstance(), "config", 
previousConfig, true);
+  }

Review Comment:
   `BackendTestExtension.BackendSetupCallback` replaces `GravitinoEnv.config` 
before each template invocation, so the capture/restore is required to avoid 
leaking global state. Kept it and added a comment documenting that lifecycle.



-- 
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]

Reply via email to