fix: modify the statement of validate semantic model sql to make its validation more comprehensive

[junjiem]

Summary by CodeRabbit

• Bug Fixes

  • Improved semantic model validation so model names with special characters are handled correctly, reducing false validation failures.

• Refactor

  • Refined the SQL validation query construction to be safer and more maintainable, improving reliability of configuration checks.

[coderabbitai]

Walkthrough

PreBuildValidator's semantic model SQL validation now constructs a quoted CTE name and uses a CTE (WITH "<quotedName>" AS (<semanticModelSql>) SELECT 1 FROM "<quotedName>" WHERE 1=0) instead of embedding the semantic model SQL as an inline subquery.

Changes

Cohort / File(s)	Summary
Semantic Model CTE Switch dat-sdk/src/main/java/ai/dat/boot/PreBuildValidator.java	Adds a local quotedName variable and replaces the inline-subquery validation SQL (SELECT 1 FROM (<semanticModelSql>) AS __dat_semantic_model WHERE 1=0) with a CTE using the quoted identifier for both CTE name and FROM clause; existing exception handling preserved.

Sequence Diagram(s)

sequenceDiagram
    participant Validator as PreBuildValidator
    participant SQL as SQLExecutor
    Note over Validator: Build validation SQL
    Validator->>Validator: quote semantic model name -> quotedName
    alt Old (inline subquery)
        Validator->>SQL: SELECT 1 FROM (<semanticModelSql>) AS __dat_semantic_model WHERE 1=0
    else New (CTE)
        Validator->>SQL: WITH "quotedName" AS (<semanticModelSql>) SELECT 1 FROM "quotedName" WHERE 1=0
    end
    SQL-->>Validator: success / parse error / execution error
    Note right of Validator: Handles exceptions unchanged

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Verify proper quoting/escaping of the CTE identifier matches target SQL dialect.
• Ensure quotedName cannot be null or malformed and is used consistently in both CTE and FROM clause.
• Confirm no behavioral changes in exception paths or logging.

Poem

🐰 I hopped through code with careful paws,
Quoted names now guard the CTE clause,
A tidy change, concise and bright,
The validator hops off into night. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: modifying the semantic model SQL validation statement to use a CTE-based approach for more comprehensive validation.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/validate_semantic_model_sql

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 56dd091 and c047bb0.

📒 Files selected for processing (1)

• dat-sdk/src/main/java/ai/dat/boot/PreBuildValidator.java (1 hunks)

🔇 Additional comments (1)

dat-sdk/src/main/java/ai/dat/boot/PreBuildValidator.java (1)

165-166: Good fix for identifier quoting; clarify the actual validation benefits claimed.

The quoted identifier properly addresses the SQL injection and syntax error concerns from the previous review. However, the PR's claim of making validation "more comprehensive" needs clarification:

• CTE vs. inline subquery: Both are syntax-checked identically by SQL engines; neither provides additional validation benefits over the other. The switch from inline subquery to CTE doesn't enhance validation coverage—the improvement comes from the quoting fix itself.
• Consistency question: This method now uses the actual semantic model name (quoted), while all other validation methods in the file (lines 120, 235, 262, 320, 388) use fixed aliases (__dat_model, __dat_semantic_model). This is safe due to quoting, but creates an inconsistency in approach.

Clarify whether the PR's "more comprehensive" claim refers specifically to the quoting fix or if there's another intended benefit to the CTE pattern. If it's only about the quoting, the PR title should reflect that accurately.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c6df9c6 and 56dd091.

📒 Files selected for processing (1)

• dat-sdk/src/main/java/ai/dat/boot/PreBuildValidator.java (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

dat-sdk/src/main/java/ai/dat/boot/PreBuildValidator.java (1)

dat-core/src/main/java/ai/dat/core/utils/SemanticModelUtil.java (1)

• SemanticModelUtil (28-154)

🔇 Additional comments (2)

dat-sdk/src/main/java/ai/dat/boot/PreBuildValidator.java (2)

166-167: Clarify how CTE pattern makes validation "more comprehensive".

The PR title states this change makes validation "more comprehensive," but both the CTE pattern and the previous subquery pattern should validate SQL syntax equally well. Could you clarify the specific validation benefit this change provides?

Additionally, this pattern is inconsistent with other validation methods in this file (lines 120, 236, 263, 321, 389) which all use fixed alias names like __dat_semantic_model. Consider maintaining consistency unless there's a compelling reason for the divergence.

---

158-158: LGTM: Local variable extraction improves clarity.

Extracting the semantic model name into a local variable is a good practice that avoids repeated method calls and improves readability.

Also applies to: 164-164