Add CAST style support and update DELETE query handling #215
Conversation
… update DELETE query handling in QueryBuilder to use EXISTS syntax
|
Caution Review failedThe pull request is closed. WalkthroughIntroduces a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SqlFormatter
participant SqlPrintTokenParser
participant CastExpression
User->>SqlFormatter: format(sql, { castStyle })
SqlFormatter->>SqlFormatter: determine castStyle (options or preset)
SqlFormatter->>SqlPrintTokenParser: new SqlPrintTokenParser({ castStyle, ... })
SqlPrintTokenParser->>CastExpression: render(node)
alt castStyle == 'postgres'
CastExpression->>CastExpression: output "input :: TYPE"
else castStyle == 'standard'
CastExpression->>CastExpression: output "CAST(input AS TYPE)"
end
CastExpression->>User: return formatted SQL
sequenceDiagram
participant QueryBuilder
participant Subquery
participant DeleteQuery
Note over QueryBuilder: Old flow (USING)
QueryBuilder->>DeleteQuery: build with usingClause
DeleteQuery->>DeleteQuery: DELETE ... USING ... WHERE ...
Note over QueryBuilder: New flow (EXISTS)
QueryBuilder->>QueryBuilder: buildEqualityPredicate()
QueryBuilder->>Subquery: wrap original selectQuery selecting 1 (LiteralValue)
QueryBuilder->>Subquery: create InlineQuery(SimpleSelectQuery)
QueryBuilder->>QueryBuilder: UnaryExpression('exists', subquery)
QueryBuilder->>DeleteQuery: build with whereClause containing EXISTS(...)
DeleteQuery->>DeleteQuery: DELETE ... WHERE EXISTS (SELECT 1 FROM ...)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/guide/formatting-recipes.md (1)
39-39: Consider consolidating CAST style documentation.The
castStyleoption is documented in two places:
- In the base formatting options table (line 39)
- As a dedicated section "Controlling CAST style" (lines 92-104)
While the detailed section provides valuable examples and migration guidance, you might want to cross-reference it from the table entry to avoid duplication. For example, the table entry could say: "Chooses how CAST expressions are printed (see 'Controlling CAST style' section below for details)."
Also applies to: 92-104
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
docs/guide/formatting-recipes.md(3 hunks)packages/core/src/parsers/SqlPrintTokenParser.ts(8 hunks)packages/core/src/transformers/QueryBuilder.ts(2 hunks)packages/core/src/transformers/SqlFormatter.ts(3 hunks)packages/core/tests/parsers/SqlPrintTokenParser.test.ts(1 hunks)packages/core/tests/parsers/ValueParser.array-slice.test.ts(3 hunks)packages/core/tests/parsers/ValueParser.test.ts(3 hunks)packages/core/tests/transformers/CTEDisabler.test.ts(1 hunks)packages/core/tests/transformers/SqlFormatter.cast-style.test.ts(1 hunks)packages/core/tests/transformers/buildDeleteQuery.test.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
packages/core/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/core/AGENTS.md)
packages/core/**/*.{ts,tsx}: Always compile and fix TypeScript errors before running tests
Source code comments must be written in English only
For model-driven mappings (mapping.typeInfo and mapping.structure), use convertModelDrivenMapping
Use imports from 'rawsql-ts' rather than local relative paths for library usage
Files:
packages/core/tests/transformers/buildDeleteQuery.test.tspackages/core/tests/transformers/CTEDisabler.test.tspackages/core/tests/parsers/ValueParser.array-slice.test.tspackages/core/src/transformers/SqlFormatter.tspackages/core/src/parsers/SqlPrintTokenParser.tspackages/core/tests/parsers/ValueParser.test.tspackages/core/tests/transformers/SqlFormatter.cast-style.test.tspackages/core/tests/parsers/SqlPrintTokenParser.test.tspackages/core/src/transformers/QueryBuilder.ts
packages/core/**/*.{test,spec}.{ts,tsx,js}
📄 CodeRabbit inference engine (packages/core/AGENTS.md)
Tests are specs — never change expected values without user consultation
Files:
packages/core/tests/transformers/buildDeleteQuery.test.tspackages/core/tests/transformers/CTEDisabler.test.tspackages/core/tests/parsers/ValueParser.array-slice.test.tspackages/core/tests/parsers/ValueParser.test.tspackages/core/tests/transformers/SqlFormatter.cast-style.test.tspackages/core/tests/parsers/SqlPrintTokenParser.test.ts
packages/core/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/core/AGENTS.md)
Remove console.log statements before commit
Files:
packages/core/tests/transformers/buildDeleteQuery.test.tspackages/core/tests/transformers/CTEDisabler.test.tspackages/core/tests/parsers/ValueParser.array-slice.test.tspackages/core/src/transformers/SqlFormatter.tspackages/core/src/parsers/SqlPrintTokenParser.tspackages/core/tests/parsers/ValueParser.test.tspackages/core/tests/transformers/SqlFormatter.cast-style.test.tspackages/core/tests/parsers/SqlPrintTokenParser.test.tspackages/core/src/transformers/QueryBuilder.ts
packages/core/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (packages/core/AGENTS.md)
Use SqlFormatter for SQL comparison tests
Files:
packages/core/tests/transformers/buildDeleteQuery.test.tspackages/core/tests/transformers/CTEDisabler.test.tspackages/core/tests/parsers/ValueParser.array-slice.test.tspackages/core/tests/parsers/ValueParser.test.tspackages/core/tests/transformers/SqlFormatter.cast-style.test.tspackages/core/tests/parsers/SqlPrintTokenParser.test.ts
🧠 Learnings (1)
📚 Learning: 2025-10-18T02:29:34.428Z
Learnt from: CR
PR: mk3008/rawsql-ts#0
File: packages/core/AGENTS.md:0-0
Timestamp: 2025-10-18T02:29:34.428Z
Learning: Applies to packages/core/**/*.{test,spec}.{ts,tsx} : Use SqlFormatter for SQL comparison tests
Applied to files:
packages/core/tests/transformers/CTEDisabler.test.tspackages/core/tests/transformers/SqlFormatter.cast-style.test.tspackages/core/tests/parsers/SqlPrintTokenParser.test.ts
🧬 Code graph analysis (4)
packages/core/src/transformers/SqlFormatter.ts (1)
packages/core/src/parsers/SqlPrintTokenParser.ts (1)
CastStyle(51-51)
packages/core/src/parsers/SqlPrintTokenParser.ts (1)
packages/core/src/models/SqlPrintToken.ts (1)
SqlPrintToken(96-126)
packages/core/tests/transformers/SqlFormatter.cast-style.test.ts (2)
packages/core/src/parsers/SelectQueryParser.ts (1)
SelectQueryParser(31-669)packages/core/src/transformers/SqlFormatter.ts (1)
SqlFormatter(114-164)
packages/core/src/transformers/QueryBuilder.ts (3)
packages/core/src/models/Clause.ts (4)
SelectClause(17-28)SelectItem(6-15)FromClause(306-336)WhereClause(49-56)packages/core/src/models/ValueComponent.ts (3)
LiteralValue(211-221)UnaryExpression(187-196)InlineQuery(29-36)packages/core/src/models/SimpleSelectQuery.ts (1)
SimpleSelectQuery(44-753)
🔇 Additional comments (18)
packages/core/tests/parsers/ValueParser.array-slice.test.ts (1)
31-31: Test expectations updated to reflect new CAST rendering.The expected outputs have been updated from PostgreSQL-style
::casts to ANSIcast(... as ...)format, which aligns with the PR's introduction of CAST style support. This is consistent with the new default behavior.Also applies to: 87-87
packages/core/tests/parsers/ValueParser.test.ts (1)
40-40: Comprehensive test expectations updated for CAST normalization.The test expectations have been systematically updated to reflect the new CAST rendering behavior, covering a wide range of scenarios including parameterized types, namespaced types, complex expressions, and array types. All updates consistently use the ANSI
cast(... as ...)format instead of PostgreSQL's::syntax.Also applies to: 61-111
packages/core/tests/parsers/SqlPrintTokenParser.test.ts (1)
133-133: Test expectation updated for namespaced type cast.The expected output correctly reflects the new ANSI CAST format for namespaced types.
packages/core/tests/transformers/SqlFormatter.cast-style.test.ts (1)
1-19: Well-structured tests for CAST style feature.This new test file properly verifies both the default ANSI CAST behavior and the PostgreSQL
::syntax when using the postgres preset. The tests useSqlFormatterfor SQL comparison as per the coding guidelines.packages/core/tests/transformers/CTEDisabler.test.ts (1)
413-413: Test expectation updated for complex analytics query.The expected SQL now correctly uses ANSI
cast(... as DECIMAL)syntax in both the UNION branches, consistent with the new CAST rendering behavior.packages/core/tests/transformers/buildDeleteQuery.test.ts (1)
18-18: DELETE query generation updated to use EXISTS-based predicates.All test expectations have been updated to reflect the new DELETE query strategy, which uses correlated EXISTS subqueries instead of PostgreSQL-specific USING clauses. This approach is more portable across different SQL databases while maintaining the same semantic behavior.
The pattern
delete from ... where exists (select 1 from (...) as "src" where ...)is consistently applied across all test cases, including those with CTEs, composite keys, and additional match columns.Also applies to: 32-32, 47-47, 60-60, 75-75
docs/guide/formatting-recipes.md (1)
92-104: Well-documented feature with clear examples.The dedicated CAST style section provides excellent guidance, including:
- Clear examples showing both formats
- Explanation of which presets enable which style
- Migration advice for moving away from PostgreSQL-only syntax
This will help users understand when and how to use the new option.
packages/core/src/transformers/SqlFormatter.ts (1)
1-1: CAST style option properly integrated.The
castStyleoption is cleanly integrated into the formatter pipeline:
- Type imported from the parser module
- Exposed in the public
SqlFormatterOptionsinterface- Properly wired through to the parser with preset fallback (line 134)
The implementation follows the same pattern as other formatter options like
parameterStyleandparameterSymbol.Also applies to: 99-100, 134-134
packages/core/src/parsers/SqlPrintTokenParser.ts (7)
51-51: LGTM!The
CastStyletype definition is clean and appropriate for representing the two supported CAST syntax styles.
63-64: LGTM!The
castStylefield is properly documented and typed, with appropriate optionality for backward compatibility.
77-77: LGTM!The
castStyle: 'postgres'is correctly configured for PostgreSQL and PostgreSQL-compatible databases (postgres, postgresWithNamedParams, cockroachdb, redshift). Other databases will appropriately default to 'standard' ANSI CAST syntax.Also applies to: 83-83, 134-134, 160-160
212-212: LGTM!The private
castStylefield is appropriately scoped and typed.
218-220: LGTM!The constructor signature is correctly extended with the optional
castStyleparameter. The inline union type forparameterStyleis also appropriate.
237-237: LGTM!The initialization correctly defaults to
'standard'for ANSI SQL compatibility, with PostgreSQL-based databases explicitly configuring'postgres'in their presets.
1322-1344: LGTM!The conditional rendering logic correctly handles both CAST styles:
- PostgreSQL style:
input::type(concise, PostgreSQL-specific)- Standard style:
CAST(input AS type)(verbose, ANSI SQL compliant)The implementation properly branches on
castStyleand both code paths are correct.packages/core/src/transformers/QueryBuilder.ts (3)
1-1: LGTM!Import changes correctly reflect the refactoring from USING clause to EXISTS-based approach. The removed
UsingClauseand addedInlineQuery,LiteralValue, andUnaryExpressionare all appropriately used in the updatedbuildDeleteQuerymethod.Also applies to: 5-5
312-321: LGTM! Well-documented architectural change.The refactoring from PostgreSQL-specific USING clause to ANSI-compliant EXISTS subquery is correctly implemented. The EXISTS subquery properly:
- Selects a literal value (1) since EXISTS only needs to test existence
- Uses the source query as a correlated subquery
- Applies the equality predicate for correlation with the DELETE target
The inline comment clearly documents the rationale for this change. While the USING clause might be more performant in PostgreSQL, the EXISTS approach ensures broader database compatibility.
323-327: LGTM!The
DeleteQueryconstruction correctly reflects the new EXISTS-based approach, passing onlydeleteClause,whereClause, andwithClause. The removal ofusingClausealigns with the architectural change to use standard SQL instead of PostgreSQL-specific syntax.
… include usage notes and examples
1 participant
Introduce support for different CAST styles in
SqlFormatterandSqlPrintTokenParser. Update theQueryBuilderto handle DELETE queries using EXISTS syntax instead of the PostgreSQL-specific USING clause.Summary by CodeRabbit
New Features
castStyleformatting option to emit either PostgreSQL::casts or ANSICAST()syntax.Bug Fixes
Documentation
castStylewith examples and migration notes.Tests