Copilot commented on code in PR #12602:
URL: https://github.com/apache/gravitino/pull/12602#discussion_r3861564625


##########
core/src/main/java/org/apache/gravitino/storage/relational/service/SemanticModelPOStorageOps.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * 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.storage.relational.service;
+
+import java.util.Locale;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.exceptions.NoSuchEntityException;
+import org.apache.gravitino.storage.relational.mapper.SemanticModelMetaMapper;
+import org.apache.gravitino.storage.relational.po.SemanticModelPO;
+
+/** Provides relational persistent-object operations required to create and 
load Semantic Models. */
+public class SemanticModelPOStorageOps
+    extends BasePOStorageOps<SemanticModelPO, SemanticModelMetaMapper> {
+
+  /** Creates Semantic Model persistent-object operations. */
+  public SemanticModelPOStorageOps() {}
+
+  @Override
+  public void insertPO(
+      SemanticModelMetaMapper mapper, SemanticModelPO semanticModelPO, boolean 
overwrite) {
+    if (overwrite) {
+      mapper.insertSemanticModelMetaOnDuplicateKeyUpdate(semanticModelPO);
+    } else {
+      mapper.insertSemanticModelMeta(semanticModelPO);
+    }
+  }
+
+  @Override
+  public SemanticModelPO getPO(
+      SemanticModelMetaMapper mapper, Long parentId, String semanticModelName) 
{
+    return mapper.selectSemanticModelMetaBySchemaIdAndName(parentId, 
semanticModelName);
+  }
+
+  @Override
+  public SemanticModelPO getPOByFullName(
+      SemanticModelMetaMapper mapper, NameIdentifier identifier) {
+    Namespace namespace = identifier.namespace();
+    SemanticModelPO po =
+        mapper.selectSemanticModelByFullQualifiedName(
+            namespace.level(0), namespace.level(1), namespace.level(2), 
identifier.name());
+    if (po == null) {
+      throw new NoSuchEntityException(
+          NoSuchEntityException.NO_SUCH_ENTITY_MESSAGE,
+          Entity.EntityType.CATALOG.name().toLowerCase(Locale.ROOT),
+          namespace.level(1));

Review Comment:
   When the metalake does not exist, 
`selectSemanticModelByFullQualifiedName(...)` returns `null` (because the SQL 
`WHERE` filters on metalake), but the code throws `NoSuchEntityException` for a 
**catalog**. This misreports the missing entity. A concrete fix is to make the 
SQL return a row even when the catalog is missing (e.g., left-join the catalog 
as well, similar to schema/model) so you can distinguish: metalake missing vs 
catalog missing vs schema missing; alternatively, if keeping the current SQL 
shape, treat `po == null` as a metalake-missing case (using 
`namespace.level(0)` and `EntityType.METALAKE`) and add a separate check/query 
for catalog existence to preserve correct error classification.



##########
core/src/main/java/org/apache/gravitino/storage/relational/po/SemanticModelVersionInfoPO.java:
##########
@@ -0,0 +1,91 @@
+/*
+ * 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.storage.relational.po;
+
+import com.google.common.base.Preconditions;
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
+import lombok.ToString;
+import lombok.experimental.Accessors;
+import org.apache.commons.lang3.StringUtils;
+
+/** The persistent object for a complete Semantic Model version snapshot. */
+@EqualsAndHashCode
+@Getter
+@ToString
+@Accessors(fluent = true)
+public class SemanticModelVersionInfoPO {
+
+  private Long id;
+  private Long metalakeId;
+  private Long catalogId;
+  private Long schemaId;
+  private Long semanticModelId;
+  private Integer version;
+  private String semanticModelName;

Review Comment:
   The snapshot PO uses `Integer version`, while the identity PO uses `Long 
currentVersion/lastVersion`. This type mismatch can lead to implicit casts in 
SQL joins/comparisons and creates an avoidable overflow boundary for long-lived 
entities. Consider standardizing version types across POs (and DB columns) to a 
single type (preferably `Long`) to keep mappings and SQL consistent.



##########
core/src/main/java/org/apache/gravitino/storage/relational/po/SemanticModelPO.java:
##########
@@ -0,0 +1,224 @@
+/*
+ * 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.storage.relational.po;
+
+import static 
org.apache.gravitino.storage.relational.utils.POConverters.DEFAULT_DELETED_AT;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.google.common.base.Preconditions;
+import java.util.Collections;
+import java.util.Map;
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
+import lombok.ToString;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.json.JsonUtils;
+import org.apache.gravitino.meta.AuditInfo;
+import org.apache.gravitino.meta.NamespacedEntityId;
+import org.apache.gravitino.meta.SemanticModelEntity;
+import org.apache.gravitino.semantic.SemanticModelDefinition;
+import org.apache.gravitino.storage.relational.service.EntityIdService;
+
+/** The persistent object for Semantic Model identity metadata and its current 
version snapshot. */
+@Getter
+@EqualsAndHashCode(exclude = "semanticModelVersionInfoPO")
+@ToString
+public class SemanticModelPO {
+
+  /** The initial version allocated to a newly created Semantic Model. */
+  public static final Long INITIAL_VERSION = 1L;
+
+  private Long semanticModelId;
+  private String semanticModelName;
+  private Long metalakeId;
+  private Long catalogId;
+  private Long schemaId;
+  private String auditInfo;
+  private Long currentVersion;
+  private Long lastVersion;
+  private Long deletedAt;
+  private SemanticModelVersionInfoPO semanticModelVersionInfoPO;
+
+  /** Creates an empty persistent object for MyBatis. */
+  public SemanticModelPO() {}
+
+  /** A Lombok builder for {@link SemanticModelPO}. */
+  public static class SemanticModelPOBuilder {
+    // Lombok generates the builder methods.
+  }
+
+  @lombok.Builder(setterPrefix = "with")
+  private SemanticModelPO(
+      Long semanticModelId,
+      String semanticModelName,
+      Long metalakeId,
+      Long catalogId,
+      Long schemaId,
+      String auditInfo,
+      Long currentVersion,
+      Long lastVersion,
+      Long deletedAt,
+      SemanticModelVersionInfoPO semanticModelVersionInfoPO) {
+    Preconditions.checkArgument(semanticModelId != null, "Semantic Model id is 
required");
+    Preconditions.checkArgument(semanticModelName != null, "Semantic Model 
name is required");
+    Preconditions.checkArgument(metalakeId != null, "Metalake id is required");
+    Preconditions.checkArgument(catalogId != null, "Catalog id is required");
+    Preconditions.checkArgument(schemaId != null, "Schema id is required");
+    Preconditions.checkArgument(auditInfo != null, "Audit info is required");
+    Preconditions.checkArgument(currentVersion != null, "Current version is 
required");
+    Preconditions.checkArgument(lastVersion != null, "Last version is 
required");
+    Preconditions.checkArgument(deletedAt != null, "Deleted at is required");
+
+    this.semanticModelId = semanticModelId;
+    this.semanticModelName = semanticModelName;
+    this.metalakeId = metalakeId;
+    this.catalogId = catalogId;
+    this.schemaId = schemaId;
+    this.auditInfo = auditInfo;
+    this.currentVersion = currentVersion;
+    this.lastVersion = lastVersion;
+    this.deletedAt = deletedAt;
+    this.semanticModelVersionInfoPO = semanticModelVersionInfoPO;
+  }
+
+  /**
+   * Converts a persistent object and its current version snapshot to a 
Semantic Model entity.
+   *
+   * @param semanticModelPO The persistent object to convert.
+   * @param namespace The Semantic Model namespace.
+   * @return The converted Semantic Model entity.
+   */
+  public static SemanticModelEntity fromSemanticModelPO(
+      SemanticModelPO semanticModelPO, Namespace namespace) {
+    try {
+      SemanticModelVersionInfoPO versionPO = 
semanticModelPO.getSemanticModelVersionInfoPO();
+      SemanticModelDefinition definition =
+          
SemanticModelDefinitionSerDe.deserialize(versionPO.semanticModelDefinition());
+      Map<String, String> properties =
+          versionPO.properties() == null
+              ? Collections.emptyMap()
+              : JsonUtils.anyFieldMapper()
+                  .readValue(
+                      versionPO.properties(),
+                      JsonUtils.anyFieldMapper()
+                          .getTypeFactory()
+                          .constructMapType(Map.class, String.class, 
String.class));
+
+      return SemanticModelEntity.builder()
+          .withId(semanticModelPO.getSemanticModelId())
+          .withName(versionPO.semanticModelName())
+          .withNamespace(namespace)
+          .withComment(versionPO.semanticModelComment())
+          .withDefinition(definition)
+          .withProperties(properties)
+          .withAuditInfo(
+              
JsonUtils.anyFieldMapper().readValue(semanticModelPO.getAuditInfo(), 
AuditInfo.class))
+          .build();
+    } catch (JsonProcessingException e) {
+      throw new RuntimeException("Failed to deserialize Semantic Model JSON", 
e);
+    }
+  }
+
+  /**
+   * Initializes a new Semantic Model identity and version-one snapshot.
+   *
+   * @param semanticModelEntity The Semantic Model entity.
+   * @param builder The identity persistent-object builder.
+   * @return The initialized persistent object.
+   */
+  public static SemanticModelPO initializeSemanticModelPO(
+      SemanticModelEntity semanticModelEntity, SemanticModelPOBuilder builder) 
{
+    
builder.withCurrentVersion(INITIAL_VERSION).withLastVersion(INITIAL_VERSION);
+    return buildSemanticModelPO(semanticModelEntity, builder, 
INITIAL_VERSION.intValue());
+  }
+
+  /**
+   * Creates a complete version snapshot for a Semantic Model entity.
+   *
+   * @param semanticModelEntity The Semantic Model entity.
+   * @param namespacedEntityId The resolved schema and ancestor IDs.
+   * @param version The version to allocate.
+   * @return The version snapshot persistent object.
+   */
+  public static SemanticModelVersionInfoPO 
initializeSemanticModelVersionInfoPO(
+      SemanticModelEntity semanticModelEntity,
+      NamespacedEntityId namespacedEntityId,
+      Integer version) {
+    try {
+      String definitionJson =
+          
SemanticModelDefinitionSerDe.serialize(semanticModelEntity.definition());
+      String propertiesJson =
+          semanticModelEntity.properties().isEmpty()
+              ? null
+              : 
JsonUtils.anyFieldMapper().writeValueAsString(semanticModelEntity.properties());
+
+      return SemanticModelVersionInfoPO.builder()
+          .withSemanticModelId(semanticModelEntity.id())
+          .withMetalakeId(namespacedEntityId.namespaceIds()[0])
+          .withCatalogId(namespacedEntityId.namespaceIds()[1])
+          .withSchemaId(namespacedEntityId.entityId())
+          .withVersion(version)
+          .withSemanticModelName(semanticModelEntity.name())
+          .withSemanticModelComment(semanticModelEntity.comment())
+          .withSemanticModelDefinition(definitionJson)
+          .withProperties(propertiesJson)
+          .withAuditInfo(
+              
JsonUtils.anyFieldMapper().writeValueAsString(semanticModelEntity.auditInfo()))
+          .withDeletedAt(DEFAULT_DELETED_AT)
+          .build();
+    } catch (JsonProcessingException e) {
+      throw new RuntimeException("Failed to serialize Semantic Model JSON", e);
+    }
+  }
+
+  /**
+   * Builds a Semantic Model identity persistent object and the requested 
complete snapshot.
+   *
+   * @param semanticModelEntity The Semantic Model entity.
+   * @param builder The identity persistent-object builder.
+   * @param version The version to allocate.
+   * @return The built persistent object.
+   */
+  public static SemanticModelPO buildSemanticModelPO(
+      SemanticModelEntity semanticModelEntity, SemanticModelPOBuilder builder, 
Integer version) {
+    try {
+      NamespacedEntityId namespacedEntityId =
+          EntityIdService.getEntityIds(
+              NameIdentifier.of(semanticModelEntity.namespace().levels()),
+              Entity.EntityType.SCHEMA);
+      SemanticModelVersionInfoPO versionPO =
+          initializeSemanticModelVersionInfoPO(semanticModelEntity, 
namespacedEntityId, version);
+      return builder
+          .withSemanticModelId(semanticModelEntity.id())
+          .withSemanticModelName(semanticModelEntity.name())
+          .withMetalakeId(namespacedEntityId.namespaceIds()[0])
+          .withCatalogId(namespacedEntityId.namespaceIds()[1])
+          .withSchemaId(namespacedEntityId.entityId())
+          .withAuditInfo(
+              
JsonUtils.anyFieldMapper().writeValueAsString(semanticModelEntity.auditInfo()))

Review Comment:
   `buildSemanticModelPO(...)` is public and does not set `currentVersion` / 
`lastVersion`, yet the PO constructor requires them (via Preconditions). This 
makes the method easy to misuse (callers can pass a fresh builder and get a 
runtime failure). To make the API harder to misuse, either: (a) set 
`currentVersion/lastVersion` inside this method when appropriate, (b) make this 
helper `private`/package-private and expose only safer factory methods (like 
`initializeSemanticModelPO`), or (c) document clearly in Javadoc that the 
caller must pre-populate those builder fields.



##########
core/src/test/java/org/apache/gravitino/storage/relational/TestSemanticModelJDBCBackend.java:
##########
@@ -0,0 +1,259 @@
+/*
+ * 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.storage.relational;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import com.google.common.collect.ImmutableMap;
+import java.io.IOException;
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.List;
+import java.util.Map;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.EntityAlreadyExistsException;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.exceptions.NoSuchEntityException;
+import org.apache.gravitino.meta.SemanticModelEntity;
+import org.apache.gravitino.semantic.AIContext;
+import org.apache.gravitino.semantic.AIContextObject;
+import org.apache.gravitino.semantic.CustomExtension;
+import org.apache.gravitino.semantic.DataType;
+import org.apache.gravitino.semantic.Dataset;
+import org.apache.gravitino.semantic.DialectExpression;
+import org.apache.gravitino.semantic.Dimension;
+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;
+import org.apache.gravitino.storage.RandomIdGenerator;
+import 
org.apache.gravitino.storage.relational.mapper.SemanticModelVersionInfoMapper;
+import org.apache.gravitino.storage.relational.po.SemanticModelPO;
+import org.apache.gravitino.storage.relational.session.SqlSessionFactoryHelper;
+import org.apache.gravitino.storage.relational.utils.SessionUtils;
+import org.apache.gravitino.utils.NamespaceUtil;
+import org.apache.ibatis.session.SqlSession;
+import org.junit.jupiter.api.TestTemplate;
+
+/** Tests Semantic Model create and load persistence through {@link 
JDBCBackend}. */
+public class TestSemanticModelJDBCBackend extends TestJDBCBackend {
+
+  @TestTemplate
+  public void testCreateAndLoadRoundTrip() throws IOException {
+    Namespace namespace = createParents("round_trip");
+    SemanticModelEntity absent =
+        semanticModel(
+            RandomIdGenerator.INSTANCE.nextId(),
+            namespace,
+            "absent_optional_model",
+            false,
+            ImmutableMap.of("domain", "sales"));
+    SemanticModelEntity empty =
+        semanticModel(
+            RandomIdGenerator.INSTANCE.nextId(),
+            namespace,
+            "empty_optional_model",
+            true,
+            ImmutableMap.of());
+
+    backend.insert(absent, false);
+    backend.insert(empty, false);
+
+    SemanticModelEntity loadedAbsent =
+        backend.get(absent.nameIdentifier(), Entity.EntityType.SEMANTIC_MODEL);
+    SemanticModelEntity loadedEmpty =
+        backend.get(empty.nameIdentifier(), Entity.EntityType.SEMANTIC_MODEL);
+
+    assertEquals(absent, loadedAbsent);
+    assertEquals(empty, loadedEmpty);
+    assertNull(loadedAbsent.definition().relationships());
+    assertNull(loadedAbsent.definition().metrics());
+    assertEquals(0, loadedEmpty.definition().relationships().length);
+    assertEquals(0, loadedEmpty.definition().metrics().length);
+    assertTrue(loadedEmpty.properties().isEmpty());
+    assertEquals(
+        new BigDecimal("1.50"),
+        
loadedAbsent.definition().aiContext().object().additionalProperties().get("threshold"));
+    assertNull(loadedAbsent.definition().aiContext().object().examples());
+    assertEquals(0, 
loadedAbsent.definition().aiContext().object().synonyms().length);
+  }
+
+  @TestTemplate
+  public void testDuplicateAndMissingEntities() throws IOException {
+    Namespace namespace = createParents("duplicate");
+    SemanticModelEntity original =
+        semanticModel(
+            RandomIdGenerator.INSTANCE.nextId(),
+            namespace,
+            "sales_model",
+            false,
+            ImmutableMap.of("owner", "analytics"));
+    backend.insert(original, false);
+
+    SemanticModelEntity duplicate =
+        semanticModel(
+            RandomIdGenerator.INSTANCE.nextId(),
+            namespace,
+            original.name(),
+            true,
+            ImmutableMap.of());
+    assertThrows(EntityAlreadyExistsException.class, () -> 
backend.insert(duplicate, false));
+    assertEquals(
+        original, backend.get(original.nameIdentifier(), 
Entity.EntityType.SEMANTIC_MODEL));
+
+    NameIdentifier missingModel = NameIdentifier.of(namespace, 
"missing_model");
+    assertThrows(
+        NoSuchEntityException.class,
+        () -> backend.get(missingModel, Entity.EntityType.SEMANTIC_MODEL));
+
+    String suffix = Long.toUnsignedString(RandomIdGenerator.INSTANCE.nextId());
+    String metalakeName = "missing_parent_metalake_" + suffix;
+    String catalogName = "missing_parent_catalog_" + suffix;
+    createAndInsertMakeLake(metalakeName);
+    createAndInsertCatalog(metalakeName, catalogName);
+    Namespace missingParentNamespace =
+        NamespaceUtil.ofSemanticModel(metalakeName, catalogName, 
"missing_schema");
+    SemanticModelEntity missingParent =
+        semanticModel(
+            RandomIdGenerator.INSTANCE.nextId(),
+            missingParentNamespace,
+            "orphan_model",
+            false,
+            ImmutableMap.of());
+
+    assertThrows(NoSuchEntityException.class, () -> 
backend.insert(missingParent, false));
+    assertEquals(0, countRows("semantic_model_meta", missingParent.id()));
+    assertEquals(0, countRows("semantic_model_version_info", 
missingParent.id()));
+  }
+
+  @TestTemplate
+  public void testCreateRollsBackIdentityWhenSnapshotInsertFails() throws 
IOException {
+    Namespace namespace = createParents("transaction");
+    SemanticModelEntity semanticModel =
+        semanticModel(
+            RandomIdGenerator.INSTANCE.nextId(),
+            namespace,
+            "transaction_model",
+            false,
+            ImmutableMap.of("domain", "finance"));
+    SemanticModelPO po =
+        SemanticModelPO.initializeSemanticModelPO(semanticModel, 
SemanticModelPO.builder());
+    SessionUtils.doWithCommit(
+        SemanticModelVersionInfoMapper.class,
+        mapper -> 
mapper.insertSemanticModelVersionInfo(po.getSemanticModelVersionInfoPO()));
+
+    assertThrows(EntityAlreadyExistsException.class, () -> 
backend.insert(semanticModel, false));
+    assertEquals(0, countRows("semantic_model_meta", semanticModel.id()));
+    assertEquals(1, countRows("semantic_model_version_info", 
semanticModel.id()));
+  }
+
+  private Namespace createParents(String prefix) throws IOException {
+    String suffix = Long.toUnsignedString(RandomIdGenerator.INSTANCE.nextId());
+    String metalakeName = prefix + "_metalake_" + suffix;
+    String catalogName = prefix + "_catalog_" + suffix;
+    String schemaName = prefix + "_schema_" + suffix;
+    createAndInsertMakeLake(metalakeName);
+    createAndInsertCatalog(metalakeName, catalogName);
+    createAndInsertSchema(metalakeName, catalogName, schemaName);
+    return NamespaceUtil.ofSemanticModel(metalakeName, catalogName, 
schemaName);
+  }
+
+  private SemanticModelEntity semanticModel(
+      Long id,
+      Namespace namespace,
+      String name,
+      boolean explicitEmpty,
+      Map<String, String> properties) {
+    AIContextObject context =
+        AIContextObject.builder()
+            .withInstructions("Use certified sales definitions")
+            .withSynonyms(new String[0])
+            .withAdditionalProperties(
+                Map.of("threshold", new BigDecimal("1.50"), "nested", 
List.of(new BigInteger("3"))))
+            .build();
+    Field field =
+        Field.builder()
+            .withName("ordered_at")
+            .withExpression(
+                Expression.builder()
+                    .withDialects(
+                        new DialectExpression[] {
+                          DialectExpression.builder()
+                              .withDialect("ansi")
+                              .withExpression("ordered_at")
+                              .build()
+                        })
+                    .build())
+            .withDimension(Dimension.builder().withIsTime(true).build())
+            .withDatatype(DataType.DATE_TIME_TZ)
+            .build();
+    Dataset dataset =
+        Dataset.builder()
+            .withName("orders")
+            .withSource(NameIdentifier.of("sales", "mart", "orders"))
+            .withPrimaryKey(new String[0])
+            .withFields(new Field[] {field})
+            .build();
+    SemanticModelDefinition.Builder definitionBuilder =
+        SemanticModelDefinition.builder()
+            .withAIContext(AIContext.of(context))
+            .withDatasets(new Dataset[] {dataset});
+    if (explicitEmpty) {
+      definitionBuilder
+          .withRelationships(new Relationship[0])
+          .withMetrics(new Metric[0])
+          .withCustomExtensions(new CustomExtension[0]);
+    }
+
+    return SemanticModelEntity.builder()
+        .withId(id)
+        .withName(name)
+        .withNamespace(namespace)
+        .withComment(explicitEmpty ? null : "Governed sales definitions")
+        .withDefinition(definitionBuilder.build())
+        .withProperties(properties)
+        .withAuditInfo(AUDIT_INFO)
+        .build();
+  }
+
+  private int countRows(String tableName, Long semanticModelId) {
+    String sql =
+        String.format(
+            "SELECT count(*) FROM %s WHERE semantic_model_id = %d", tableName, 
semanticModelId);
+    try (SqlSession sqlSession =
+            
SqlSessionFactoryHelper.getInstance().getSqlSessionFactory().openSession(true);
+        Connection connection = sqlSession.getConnection();
+        Statement statement = connection.createStatement();
+        ResultSet resultSet = statement.executeQuery(sql)) {

Review Comment:
   This test helper builds SQL via string formatting and executes it with a 
plain `Statement`. Even in tests, prefer parameter binding (e.g., 
`PreparedStatement`) for correctness and to avoid accidental SQL issues if the 
helper is reused/extended. If you keep `tableName` dynamic, consider 
constraining it to a fixed allowlist (the specific tables used in this test) to 
prevent accidental injection via future refactors.



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