feat(core): add optimizer for unused left joins and CTEs #393
Conversation
📝 WalkthroughWalkthroughThis PR introduces schema-aware query optimization capabilities to DynamicQueryBuilder, enabling automated removal of unused LEFT JOINs and unused CTEs from SQL queries. A new OptimizeUnusedLeftJoins transformer module provides the optimization logic, integrated into the builder via QueryBuildOptions. New public functions and SchemaInfo types are exported for external use, with comprehensive test coverage and documentation. Changes
Sequence DiagramsequenceDiagram
participant Client
participant DynamicQueryBuilder
participant OptimizeUnusedLeftJoins
participant AST
Client->>DynamicQueryBuilder: buildQuery(options with<br/>schemaInfo & pruning flags)
DynamicQueryBuilder->>DynamicQueryBuilder: Build initial query AST
alt removeUnusedLeftJoins enabled
DynamicQueryBuilder->>OptimizeUnusedLeftJoins: optimizeUnusedLeftJoinsToFixedPoint(query, schemaInfo)
OptimizeUnusedLeftJoins->>AST: Analyze schema & traverse joins
OptimizeUnusedLeftJoins->>AST: Identify unused LEFT JOINs<br/>(via unique key analysis)
OptimizeUnusedLeftJoins->>AST: Remove unused joins (fixed-point)
OptimizeUnusedLeftJoins-->>DynamicQueryBuilder: Optimized query
end
alt removeUnusedCtes enabled
DynamicQueryBuilder->>OptimizeUnusedLeftJoins: optimizeUnusedCtesToFixedPoint(query)
OptimizeUnusedLeftJoins->>AST: Analyze CTE references
OptimizeUnusedLeftJoins->>AST: Identify unused CTEs<br/>(non-recursive, unused by main/other CTEs)
OptimizeUnusedLeftJoins->>AST: Remove unused CTEs (cascading, fixed-point)
OptimizeUnusedLeftJoins-->>DynamicQueryBuilder: Optimized query
end
DynamicQueryBuilder->>DynamicQueryBuilder: Serialize to JSON
DynamicQueryBuilder-->>Client: Final optimized query
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes The review requires careful analysis of the new AST traversal logic in OptimizeUnusedLeftJoins (454 lines of dense optimization code with schema-aware analysis and recursive query handling), validation of integration points in DynamicQueryBuilder, edge case verification across multiple join and CTE scenarios, and confirmation that fixed-point iteration logic correctly handles cascading deletions. Possibly Related Issues
Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. 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/querybuilding-recipes.md (1)
114-122: LGTM!The documentation clearly explains both pruning options, their prerequisites (
schemaInfofor left joins), and the AST-driven approach. The content aligns well with the implementation.Consider adding a brief code example showing how to enable pruning:
const query = builder.buildQuery(baseSql, { removeUnusedLeftJoins: true, removeUnusedCtes: true, schemaInfo: [ { name: 'profiles', columns: ['id', 'user_id'], uniqueKeys: [['id']] } ] });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/document-optimizer-prune.mddocs/guide/querybuilding-recipes.mdpackages/core/src/index.tspackages/core/src/transformers/DynamicQueryBuilder.tspackages/core/src/transformers/OptimizeUnusedLeftJoins.tspackages/core/tests/transformers/DynamicQueryBuilder.test.ts
🧰 Additional context used
📓 Path-based instructions (5)
packages/core/src/**/*.ts
📄 CodeRabbit inference engine (packages/core/AGENTS.md)
packages/core/src/**/*.ts: Ensure TypeScript errors stay at zero before running tests
Keep all comments and identifiers in English
Always prefer AST helpers (SelectQueryParser, SelectAnalyzer, splitQueries, formatter APIs) for SQL parsing
Regex-based rewrites are allowed only as explicit fallbacks with a comment explaining the limitation and a tracking issue reference
When triaging parser/formatter bugs, prefer extending tokens, visitors, or AST nodes instead of layering new regex filters outside AST
Remove console.log, debugger statements, and tracing hooks before submitting a PR
Do not embed driver-level, DB-level, or testkit-level semantics inside the core engine; AST may hold vendor constructs, but behavior is decided by formatters or higher layers
Do not perform or simulate query rewriting inside core; query rewriting belongs to testkit-core
Do not introduce dialect branching (if postgres...) in parser or analyzer logic
Avoid premature optimization that harms readability or AST clarity
Refactor for clarity and document public APIs before running the validation pipeline
Use convertModelDrivenMapping when mapping.typeInfo && mapping.structure exist; legacy mode expects mapping.rootName && mapping.rootEntity
Prefer import { ... } from 'rawsql-ts' over deep relative paths
Files:
packages/core/src/index.tspackages/core/src/transformers/OptimizeUnusedLeftJoins.tspackages/core/src/transformers/DynamicQueryBuilder.ts
{packages/core,packages/testkit-core,packages/pg-testkit,packages/sqlite-testkit}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
All SQL rewrites must rely on
rawsql-tsAST utilities (parser, analyzer, splitQueries). Regex-based rewrites are allowed only as guarded fallbacks with comments explaining why. Block contributions introducing regex parsing when an AST alternative exists.
Files:
packages/core/src/index.tspackages/core/src/transformers/OptimizeUnusedLeftJoins.tspackages/core/src/transformers/DynamicQueryBuilder.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Remove console debugging before committing.
Files:
packages/core/src/index.tspackages/core/tests/transformers/DynamicQueryBuilder.test.tspackages/core/src/transformers/OptimizeUnusedLeftJoins.tspackages/core/src/transformers/DynamicQueryBuilder.ts
**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.{ts,tsx}: All exported classes, functions, and types insrc/are considered part of the public API and must have clear English JSDoc attached to their declaration.
When adding a new exported symbol insrc/(class, function, type, interface, enum, etc.), always add English JSDoc in the same commit that briefly explains the role of the API and how to consume it.
When modifying an exported symbol, update the existing docstring to keep it truthful and never delete a docstring merely to keep the diff small.
If a helper is not meant to be public, make it non-exported or add@internalto its docstring rather than leaving an undocumented export.
Files:
packages/core/src/index.tspackages/core/src/transformers/OptimizeUnusedLeftJoins.tspackages/core/src/transformers/DynamicQueryBuilder.ts
packages/core/**/*.{test,spec}.ts
📄 CodeRabbit inference engine (packages/core/AGENTS.md)
packages/core/**/*.{test,spec}.ts: Tests act as specifications - never update expectations without intent and alignment
Add or update tests whenever adding features or fixing bugs
Follow TDD cycle: Red (failing test) → Compile (fix TypeScript errors) → Green (minimum implementation) → Refactor (clean code) → Verify (intentionally break test)
Use SQL normalization via SqlFormatter when comparing SQL in tests instead of hand-written comparisons
Regression testing for complex SQL should achieve ≥95% comment preservation, no token splits like table. /* comment */ column, and CASE expression comments preserved in evaluation order
Files:
packages/core/tests/transformers/DynamicQueryBuilder.test.ts
🧠 Learnings (38)
📓 Common learnings
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/testkit-core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:58:15.347Z
Learning: Applies to packages/testkit-core/src/**/*.{ts,tsx} : Always use SelectQueryParser, SelectAnalyzer, and splitQueries for rewriting SQL; use regex only as a fallback with a comment explaining the need, a link to a tracking issue, and a description of AST migration
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: playgrounds/ztd-playground/ztd/AGENTS.md:0-0
Timestamp: 2025-12-13T04:10:32.136Z
Learning: Applies to playgrounds/ztd-playground/ztd/ztd/domain-specs/**/*.md : Do not reorder or optimize SQL in domain-specs unless explicitly instructed; preserve semantic meaning as given.
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:21.022Z
Learning: Applies to packages/core/src/**/*.ts : Avoid premature optimization that harms readability or AST clarity
📚 Learning: 2025-12-02T22:58:15.347Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/testkit-core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:58:15.347Z
Learning: Applies to packages/testkit-core/src/**/*.{ts,tsx} : Always use SelectQueryParser, SelectAnalyzer, and splitQueries for rewriting SQL; use regex only as a fallback with a comment explaining the need, a link to a tracking issue, and a description of AST migration
Applied to files:
.changeset/document-optimizer-prune.mdpackages/core/src/index.tspackages/core/tests/transformers/DynamicQueryBuilder.test.tspackages/core/src/transformers/OptimizeUnusedLeftJoins.tspackages/core/src/transformers/DynamicQueryBuilder.ts
📚 Learning: 2025-12-08T14:13:45.136Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T14:13:45.136Z
Learning: Rebuild the browser bundle when parser/formatter changes using `pnpm --filter rawsql-ts build:browser`, then re-bundle for the docs demo using esbuild, and commit updated `docs/public/demo/vendor/rawsql.browser.js`.
Applied to files:
.changeset/document-optimizer-prune.md
📚 Learning: 2025-12-13T04:10:32.136Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: playgrounds/ztd-playground/ztd/AGENTS.md:0-0
Timestamp: 2025-12-13T04:10:32.136Z
Learning: Applies to playgrounds/ztd-playground/ztd/ztd/domain-specs/**/*.md : Do not reorder or optimize SQL in domain-specs unless explicitly instructed; preserve semantic meaning as given.
Applied to files:
.changeset/document-optimizer-prune.md
📚 Learning: 2025-12-08T14:13:45.136Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T14:13:45.136Z
Learning: Applies to {packages/core,packages/testkit-core,packages/pg-testkit,packages/sqlite-testkit}/src/**/*.{ts,tsx} : All SQL rewrites must rely on `rawsql-ts` AST utilities (parser, analyzer, splitQueries). Regex-based rewrites are allowed only as guarded fallbacks with comments explaining why. Block contributions introducing regex parsing when an AST alternative exists.
Applied to files:
.changeset/document-optimizer-prune.mdpackages/core/src/transformers/OptimizeUnusedLeftJoins.ts
📚 Learning: 2025-12-02T22:58:15.347Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/testkit-core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:58:15.347Z
Learning: Applies to packages/testkit-core/tests/**/*.{test,spec}.{ts,tsx} : Add test coverage for fixture resolution paths, CRUD rewrite transformations, CTE + multi-statement handling, fallback logic, identifier casing rules, and error diagnostics in testkit-core
Applied to files:
.changeset/document-optimizer-prune.mdpackages/core/src/index.tspackages/core/tests/transformers/DynamicQueryBuilder.test.tspackages/core/src/transformers/OptimizeUnusedLeftJoins.tspackages/core/src/transformers/DynamicQueryBuilder.ts
📚 Learning: 2025-12-08T14:13:45.136Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T14:13:45.136Z
Learning: Applies to {packages/pg-testkit,packages/sqlite-testkit}/**/*.test.{ts,tsx} : Do not hand-construct `QueryResult` or mock `Client#query`. All tests must flow through the rewrite pipeline + fixtures.
Applied to files:
.changeset/document-optimizer-prune.mdpackages/core/tests/transformers/DynamicQueryBuilder.test.ts
📚 Learning: 2025-12-02T22:58:15.347Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/testkit-core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:58:15.347Z
Learning: Applies to packages/testkit-core/src/**/*.{ts,tsx} : In rawsql-ts/testkit-core, remain DBMS-agnostic with no Postgres/SQLite conditionals or behavior
Applied to files:
.changeset/document-optimizer-prune.mdpackages/core/src/transformers/DynamicQueryBuilder.ts
📚 Learning: 2025-12-02T22:57:21.022Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:21.022Z
Learning: Applies to packages/core/**/*.{test,spec}.ts : Use SQL normalization via SqlFormatter when comparing SQL in tests instead of hand-written comparisons
Applied to files:
.changeset/document-optimizer-prune.md
📚 Learning: 2025-12-02T22:58:15.347Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/testkit-core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:58:15.347Z
Learning: Applies to packages/testkit-core/src/**/*.{ts,tsx} : Do not rewrite SQL by string concatenation without AST in testkit-core
Applied to files:
.changeset/document-optimizer-prune.mdpackages/core/src/transformers/OptimizeUnusedLeftJoins.ts
📚 Learning: 2025-12-02T22:58:15.347Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/testkit-core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:58:15.347Z
Learning: Applies to packages/testkit-core/src/**/*.{ts,tsx} : warn and passthrough modes in testkit-core must behave predictably and never silently rewrite incorrect SQL
Applied to files:
.changeset/document-optimizer-prune.md
📚 Learning: 2025-12-02T22:57:55.637Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/drivers/sqlite-testkit/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:55.637Z
Learning: Applies to packages/drivers/sqlite-testkit/src/**/*.{ts,tsx} : Validate fixtures at construction time to keep per-query interception fast; respect passthrough tables and wildcard overrides driven by testkit-core
Applied to files:
.changeset/document-optimizer-prune.mdpackages/core/tests/transformers/DynamicQueryBuilder.test.ts
📚 Learning: 2025-12-02T22:57:55.637Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/drivers/sqlite-testkit/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:55.637Z
Learning: Applies to packages/drivers/sqlite-testkit/src/**/*.{ts,tsx} : Do not use regex-based SQL parsing inside sqlite-testkit; if temporary regex-based merge is absolutely required, add TODO and inline comment, emit logger.debug notice, and file an issue in testkit-core
Applied to files:
.changeset/document-optimizer-prune.md
📚 Learning: 2025-12-02T22:57:21.022Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:21.022Z
Learning: Applies to packages/core/src/**/*.ts : Refactor for clarity and document public APIs before running the validation pipeline
Applied to files:
packages/core/src/index.ts
📚 Learning: 2025-12-02T22:57:21.022Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:21.022Z
Learning: Applies to packages/core/src/**/*.ts : Avoid premature optimization that harms readability or AST clarity
Applied to files:
packages/core/src/index.tspackages/core/src/transformers/OptimizeUnusedLeftJoins.ts
📚 Learning: 2025-12-02T22:57:21.022Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:21.022Z
Learning: Applies to packages/core/src/**/*.ts : Do not introduce dialect branching (if postgres...) in parser or analyzer logic
Applied to files:
packages/core/src/index.tspackages/core/src/transformers/DynamicQueryBuilder.ts
📚 Learning: 2025-12-02T22:58:15.347Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/testkit-core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:58:15.347Z
Learning: Applies to packages/testkit-core/src/**/*.{ts,tsx} : RETURNING clauses should be rendered solely as SELECT projection in testkit-core rewrites
Applied to files:
packages/core/src/index.tspackages/core/tests/transformers/DynamicQueryBuilder.test.tspackages/core/src/transformers/OptimizeUnusedLeftJoins.ts
📚 Learning: 2025-12-02T22:58:15.347Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/testkit-core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:58:15.347Z
Learning: Applies to packages/testkit-core/src/**/*.{ts,tsx} : INSERT rewrites should convert to SELECT ... FROM (VALUES fixture_rows) with correct RETURNING projection
Applied to files:
packages/core/src/index.ts
📚 Learning: 2025-12-02T22:58:15.347Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/testkit-core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:58:15.347Z
Learning: Applies to packages/testkit-core/src/**/*.{ts,tsx} : Do not use hand-written QueryResult-like objects in testkit-core; use database engine execution for final SELECT results
Applied to files:
packages/core/src/index.tspackages/core/tests/transformers/DynamicQueryBuilder.test.tspackages/core/src/transformers/OptimizeUnusedLeftJoins.tspackages/core/src/transformers/DynamicQueryBuilder.ts
📚 Learning: 2025-12-02T22:57:21.022Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:21.022Z
Learning: Applies to packages/core/src/**/*.ts : Always prefer AST helpers (SelectQueryParser, SelectAnalyzer, splitQueries, formatter APIs) for SQL parsing
Applied to files:
packages/core/src/index.tspackages/core/src/transformers/OptimizeUnusedLeftJoins.tspackages/core/src/transformers/DynamicQueryBuilder.ts
📚 Learning: 2025-12-02T22:57:21.022Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:21.022Z
Learning: Applies to packages/core/src/**/*.ts : Do not embed driver-level, DB-level, or testkit-level semantics inside the core engine; AST may hold vendor constructs, but behavior is decided by formatters or higher layers
Applied to files:
packages/core/src/index.ts
📚 Learning: 2025-12-30T00:46:45.672Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: playgrounds/ztd-playground/AGENTS.md:0-0
Timestamp: 2025-12-30T00:46:45.672Z
Learning: Applies to playgrounds/ztd-playground/tests/**/*.test.ts : Import `ZtdConfig`, `ZtdRowShapes`, `ZtdTableName`, and `tableFixture()` from `tests/generated/ztd-row-map.generated.ts` for typed fixtures
Applied to files:
packages/core/src/index.ts
📚 Learning: 2025-12-02T22:58:15.347Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/testkit-core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:58:15.347Z
Learning: Applies to packages/testkit-core/src/**/*.{ts,tsx} : FixtureStore.describeColumns must surface table names, available columns, and suggested hints for error diagnostics
Applied to files:
packages/core/src/index.ts
📚 Learning: 2025-12-13T04:09:30.398Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/drivers/pg-testkit/AGENTS.md:0-0
Timestamp: 2025-12-13T04:09:30.398Z
Learning: Load DDL-based fixtures from canonical schema files (tests/generated/ztd-row-map.generated.ts, ztd/ddl/, or legacy ddl/ directory) rather than reverse-engineering the database structure
Applied to files:
packages/core/src/index.ts
📚 Learning: 2025-12-02T22:58:15.347Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/testkit-core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:58:15.347Z
Learning: Applies to packages/testkit-core/src/**/*.{ts,tsx} : Throw descriptive fail-fast errors for missing fixtures, unknown tables/columns, ambiguous columns, and unsupported constructs (e.g., INSERT … ON CONFLICT, mutating CTEs)
Applied to files:
packages/core/src/index.tspackages/core/tests/transformers/DynamicQueryBuilder.test.ts
📚 Learning: 2025-12-02T22:58:15.347Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/testkit-core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:58:15.347Z
Learning: Applies to packages/testkit-core/src/**/*.{ts,tsx} : Use SqliteValuesBuilder (or similar values builder) for deterministic CTE VALUES creation in testkit-core; never build VALUES lists manually with string interpolation
Applied to files:
packages/core/tests/transformers/DynamicQueryBuilder.test.tspackages/core/src/transformers/DynamicQueryBuilder.ts
📚 Learning: 2025-12-02T22:57:55.637Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/drivers/sqlite-testkit/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:55.637Z
Learning: Applies to packages/drivers/sqlite-testkit/src/**/*.{ts,tsx,js} : All SQL must flow through the testkit-core AST rewriter before execution; new rewrite behavior must be added to testkit-core first, then threaded into sqlite-testkit
Applied to files:
packages/core/tests/transformers/DynamicQueryBuilder.test.tspackages/core/src/transformers/OptimizeUnusedLeftJoins.ts
📚 Learning: 2025-12-02T22:57:55.637Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/drivers/sqlite-testkit/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:55.637Z
Learning: Applies to packages/drivers/sqlite-testkit/src/**/*.{ts,tsx} : Intercept prepare, all, get, and run APIs to apply AST-based rewrite from testkit-core and execute the resulting SELECT against better-sqlite3
Applied to files:
packages/core/tests/transformers/DynamicQueryBuilder.test.tspackages/core/src/transformers/OptimizeUnusedLeftJoins.ts
📚 Learning: 2025-12-31T15:57:27.437Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: benchmarks/sql-unit-test/AGENTS.md:0-0
Timestamp: 2025-12-31T15:57:27.437Z
Learning: Applies to benchmarks/sql-unit-test/benchmarks/**/*.{js,ts,mjs,mts} : Repositories, fixtures, and testkit usage must be shared with the corresponding Vitest tests in benchmark implementations
Applied to files:
packages/core/tests/transformers/DynamicQueryBuilder.test.ts
📚 Learning: 2026-01-09T15:06:00.377Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/ztd-cli/templates/tests/AGENTS.md:0-0
Timestamp: 2026-01-09T15:06:00.377Z
Learning: Applies to packages/ztd-cli/templates/tests/tests/**/*.{ts,tsx} : Do not assume Vitest globals are available. Explicitly import `describe`, `it`, and `expect` from `vitest` unless the project explicitly documents global usage.
Applied to files:
packages/core/tests/transformers/DynamicQueryBuilder.test.ts
📚 Learning: 2025-12-02T22:57:55.637Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/drivers/sqlite-testkit/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:55.637Z
Learning: Applies to packages/drivers/sqlite-testkit/src/**/*.{ts,tsx} : Do not rely on real table state between queries even for in-memory databases; all perceived state must originate from fixtures supplied to the driver
Applied to files:
packages/core/tests/transformers/DynamicQueryBuilder.test.ts
📚 Learning: 2025-12-30T00:46:45.672Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: playgrounds/ztd-playground/AGENTS.md:0-0
Timestamp: 2025-12-30T00:46:45.672Z
Learning: Applies to playgrounds/ztd-playground/tests/**/*.test.ts : Provide explicit fixtures for each test using `tableFixture('schema.table', [{ ... }])`
Applied to files:
packages/core/tests/transformers/DynamicQueryBuilder.test.ts
📚 Learning: 2025-12-31T15:57:27.437Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: benchmarks/sql-unit-test/AGENTS.md:0-0
Timestamp: 2025-12-31T15:57:27.437Z
Learning: Applies to benchmarks/sql-unit-test/benchmarks/**/*.{js,ts,mjs,mts} : Do not bypass existing test files; reuse test fixtures and testkit setup from the corresponding Vitest tests
Applied to files:
packages/core/tests/transformers/DynamicQueryBuilder.test.ts
📚 Learning: 2025-12-02T22:57:55.637Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/drivers/sqlite-testkit/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:55.637Z
Learning: Applies to packages/drivers/sqlite-testkit/src/**/*.{ts,tsx} : Surface clear, typed errors from the driver layer using error classes like MissingFixtureError and SchemaValidationError with descriptive messages including table/column hints
Applied to files:
packages/core/tests/transformers/DynamicQueryBuilder.test.ts
📚 Learning: 2025-12-02T22:58:15.347Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/testkit-core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:58:15.347Z
Learning: The core role of rawsql-ts/testkit-core is to rewrite all CRUD SQL into fixture-backed SELECT queries without creating, reading, or mutating physical tables
Applied to files:
packages/core/tests/transformers/DynamicQueryBuilder.test.ts
📚 Learning: 2025-12-02T22:57:21.022Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:57:21.022Z
Learning: Applies to packages/core/src/**/*.ts : Do not perform or simulate query rewriting inside core; query rewriting belongs to testkit-core
Applied to files:
packages/core/src/transformers/DynamicQueryBuilder.ts
📚 Learning: 2025-12-02T22:58:15.347Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: packages/testkit-core/AGENTS.md:0-0
Timestamp: 2025-12-02T22:58:15.347Z
Learning: Applies to packages/testkit-core/src/**/*.{ts,tsx} : Add metadata to SelectAnalyzer instead of re-parsing or creating ad-hoc regex logic when extending rewrite capability in testkit-core
Applied to files:
packages/core/src/transformers/DynamicQueryBuilder.ts
📚 Learning: 2025-12-08T14:13:45.136Z
Learnt from: CR
Repo: mk3008/rawsql-ts PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T14:13:45.136Z
Learning: Applies to packages/pg-testkit/src/**/*.{ts,tsx} : Application SQL may freely use normal CRUD (`INSERT`, `UPDATE`, `DELETE`). pg-testkit will automatically rewrite them into `SELECT` queries. Library code must never bypass the rewriter.
Applied to files:
packages/core/src/transformers/DynamicQueryBuilder.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-test (20.x)
🔇 Additional comments (14)
.changeset/document-optimizer-prune.md (1)
1-4: LGTM!The changeset correctly documents the documentation and test stabilization changes with an appropriate patch version bump.
packages/core/src/index.ts (1)
76-82: LGTM!The new exports follow the established pattern in this file and correctly expose the schema-aware optimization utilities as part of the public API.
packages/core/tests/transformers/DynamicQueryBuilder.test.ts (3)
7-13: LGTM!Well-structured test fixtures that cover both the happy path (with unique keys) and the edge case (without unique keys) for schema-aware optimization.
1012-1147: LGTM!Comprehensive test coverage for the LEFT JOIN optimizer including edge cases like cascading removal, reference detection, unique key requirements, and both sides of the equality expression. The tests properly use
SqlFormatterfor SQL comparison as per coding guidelines.
1149-1216: LGTM!Excellent test coverage for the CTE optimizer including:
- Basic unused CTE removal
- Data-modifying CTE preservation (with RETURNING)
- Recursive CTE preservation
- Cascading chained CTE removal
The tests correctly verify that only safe-to-remove CTEs are pruned.
packages/core/src/transformers/DynamicQueryBuilder.ts (3)
146-157: LGTM!The new
DynamicQueryBuilderOptionsinterface is well-documented with JSDoc and provides a clean way to configure builder-level defaults while maintaining backward compatibility with the legacy function-based resolver.
177-186: LGTM!The constructor correctly handles both the legacy function-based resolver and the new options object pattern, maintaining backward compatibility while enabling the new schema-aware features.
267-275: LGTM!The optimization passes are correctly integrated:
- LEFT JOIN pruning runs only when both
removeUnusedLeftJoinsis true and schema info is available- CTE pruning runs independently when
removeUnusedCtesis true (no schema info required)- Both run before serialization, which is the correct order
The null-safe check
effectiveSchemaInfo?.lengthensures the optimizer isn't called with empty schema info.packages/core/src/transformers/OptimizeUnusedLeftJoins.ts (6)
8-24: LGTM!Well-documented public types with clear JSDoc explaining:
SchemaTableInfo: captures table metadata for safe JOIN evaluationSchemaInfo: ordered collection of table metadataThe type design is clean and provides sufficient information for uniqueness-based JOIN removal.
81-118: LGTM!The reference metadata collection correctly uses AST-based
ColumnReferenceCollectorto track:
- Namespace (table/alias) reference counts
- Unqualified column names (to prevent unsafe removal)
- Per-join condition reference counts (to isolate join-internal references)
This aligns with the coding guidelines requiring AST helpers over regex.
202-261: LGTM!The
shouldRemoveJoinfunction implements a conservative, safe approach with multiple guard clauses:
- Only LEFT JOINs (not other types)
- Not LATERAL joins
- Only plain TableSource (not subqueries)
- Schema info must exist for the table
- No external references to the join's alias
- Single equality condition only
- Unique key must cover the join column
This ensures only semantically safe removals occur.
325-335: LGTM!The fixed-point optimization is correctly implemented. The loop is guaranteed to terminate because:
- Each iteration can only remove joins (never add)
- The number of joins is finite
- Once no more removals are possible,
changedbecomesfalseThis handles cascading dependencies where removing one join makes another removable.
370-417: LGTM!The CTE optimization logic is safe and correct:
- Line 373: Skips recursive WITH clauses entirely (they may have side effects)
- Lines 395-398: Only processes SELECT-based CTEs (implicitly preserving INSERT/UPDATE/DELETE with RETURNING)
- Line 413: Uses
query.removeCTE()to maintain internal cache consistency- Reference checking includes both main query and inter-CTE dependencies
The conservative approach ensures only demonstrably unused SELECT CTEs are removed.
314-320: LGTM!All four public optimization functions have appropriate JSDoc documentation explaining their purpose. The documentation correctly describes the AST-based approach and schema metadata requirements.
mk3008
changed the title
mk3008
deleted the
381-optimize-unused-left-join-removal-safe-simple-rule-only
branch
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.