feat: Enhance QueryBuilder with DELETE and MERGE query support #214
Conversation
- Added methods to build DELETE and MERGE queries in QueryBuilder. - Implemented normalization functions for DELETE and MERGE options. - Updated existing methods to hoist CTEs and handle explicit columns. - Introduced tests for buildDeleteQuery, buildInsertQuery, and buildMergeQuery to ensure correctness. - Refactored existing update and insert query methods to improve column handling and error messaging.
WalkthroughThis PR introduces query conversion capabilities, allowing SELECT queries to transform into INSERT, UPDATE, DELETE, and MERGE queries through new conversion methods and options interfaces. It includes QueryBuilder expansions, a new utility for SELECT-to-VALUES transformations, and comprehensive test coverage across all new conversion paths. Changes
Sequence DiagramsequenceDiagram
participant User
participant SelectQ as SimpleSelectQuery<br/>BinarySelectQuery<br/>ValuesQuery
participant QB as QueryBuilder
participant Conv as Converters<br/>(Insert/Update/Delete<br/>Merge Logic)
participant Result as Result Query<br/>(Insert/Update/Delete<br/>Merge)
User->>SelectQ: toInsertQuery(options)
SelectQ->>QB: buildInsertQuery(selectQuery, options)
QB->>Conv: Column inference & CTE handling
Conv-->>QB: Normalized columns
QB->>Result: Create InsertQuery
Result-->>User: InsertQuery
User->>SelectQ: toMergeQuery(options)
SelectQ->>QB: buildMergeQuery(selectQuery, options)
QB->>Conv: Merge plan computation<br/>(update/insert/delete actions)
Conv-->>QB: When clauses & conditions
QB->>Result: Create MergeQuery
Result-->>User: MergeQuery
Note over QB,Conv: Similar flows for<br/>toUpdateQuery & toDeleteQuery
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~40 minutes Rationale: The PR spans five interdependent source files with new methods added consistently across query classes, plus a sophisticated new utility transformer. QueryBuilder undergoes significant expansion with complex option-handling logic, column inference, and merge/delete operations. However, much of the work is repetitive (same conversion methods added to multiple classes), and the heterogeneity is moderate—distinct concerns (options interfaces, delegation, builder logic, transformers, tests) are cleanly separated. Requires careful review of builder logic, column matching, and error handling, but patterns are largely consistent. Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 3
🧹 Nitpick comments (16)
packages/core/tests/transformers/buildInsertQuery.test.ts (1)
7-7: Add one more negative test for wildcard/unnamed columns.Add a case like
SELECT * FROM torSELECT 1(no alias) to assert the error: “Columns cannot be inferred from the selectQuery...”. This guards regressions in column inference. Based on learnings.packages/core/tests/transformers/buildDeleteQuery.test.ts (1)
7-7: Suggest a couple of extra cases for resilience.
- CTE hoisting:
WITH c AS (...) SELECT ...→ ensure WITH is promoted above DELETE.- Composite PKs and duplicate column names in
columns: verify de-duplication and predicate order.- Omit
sourceAliasto verify default'src'.packages/core/tests/transformers/buildUpdateQuery.test.ts (1)
7-7: Add one more negative test and a tiny cleanup.
- Negative: when the SELECT only contains primary keys (no non-PK columns), expect “No updatable columns found...”. This locks the boundary.
- Optional: extract a small
formatSql(q)helper to reduce repetition.packages/core/tests/transformers/buildMergeQuery.test.ts (2)
8-8: Optionally add one end-to-end SQL assertion.Add a single case that formats a MERGE to SQL via SqlFormatter and asserts the string. It complements the AST checks and catches formatter regressions. Based on learnings.
113-124: Future polish: clarify the insert‑columns error message.When
insertColumnsare provided but none match the SELECT output, the current error reads as if columns weren’t provided. Consider a distinct message (e.g., “Provided insertColumns do not exist in selectQuery output”). Not a blocker; test can remain as is.packages/core/tests/transformers/InsertQuerySelectValuesConverter.test.ts (1)
8-17: Add two more negative cases for stricter guarantees.
- SELECT→VALUES with FROM or WHERE should throw (guard already exists).
- VALUES→SELECT with a tuple whose length doesn’t match the column list should throw.
packages/core/src/models/SimpleSelectQuery.ts (1)
133-147: Document side effects of conversion methods (they may reorder/mutate selectClause)QueryBuilder’s builders can mutate the current SimpleSelectQuery (e.g., column reordering). Add a brief note to each toXQuery JSDoc, or consider invoking the builders on a defensive clone to avoid surprising changes to
this.packages/core/src/models/ValuesQuery.ts (1)
1-19: Remove unused import
ParameterCollectorisn’t used in this file; drop it to keep imports clean.-import { ParameterCollector } from "../transformers/ParameterCollector";packages/core/src/models/BinarySelectQuery.ts (2)
2-9: Drop unused type import
CTEOptionsisn’t used here.-import type { - SelectQuery, - CTEOptions, - InsertQueryConversionOptions, - UpdateQueryConversionOptions, - DeleteQueryConversionOptions, - MergeQueryConversionOptions -} from "./SelectQuery"; +import type { + SelectQuery, + InsertQueryConversionOptions, + UpdateQueryConversionOptions, + DeleteQueryConversionOptions, + MergeQueryConversionOptions +} from "./SelectQuery";
14-14: Remove unused import
ParameterCollectoris not referenced.-import { ParameterCollector } from "../transformers/ParameterCollector";packages/core/src/transformers/InsertQuerySelectValuesConverter.ts (3)
1-7: Prefer type-only import and direct model imports to avoid cyclesUse a type-only import for
SelectQuery, and importSimpleSelectQuery/BinarySelectQueryfrom their modules to reduce barrel-induced cycles.-import { SimpleSelectQuery, BinarySelectQuery } from "../models/SelectQuery"; +import { SimpleSelectQuery } from "../models/SimpleSelectQuery"; +import { BinarySelectQuery } from "../models/BinarySelectQuery"; -import { SelectQuery } from "../models/SelectQuery"; +import type { SelectQuery } from "../models/SelectQuery";
79-102: Strengthen SELECT→VALUES validation and alias handling
- Reject extra clauses beyond FROM/WHERE (e.g., GROUP BY, HAVING, WINDOW, ORDER, LIMIT/OFFSET, FETCH, FOR).
- Detect duplicate aliases; require a 1:1 mapping to
columnNames.- const tuples = simpleQueries.map(query => { - if (query.fromClause || (query.whereClause && query.whereClause.condition)) { - throw new Error("SELECT queries with FROM or WHERE clauses cannot be converted to VALUES."); - } + const tuples = simpleQueries.map(query => { + const hasProhibitedClauses = + !!query.fromClause || + !!query.whereClause || + !!query.groupByClause || + !!query.havingClause || + !!query.windowClause || + !!query.orderByClause || + !!query.limitClause || + !!query.offsetClause || + !!query.fetchClause || + !!query.forClause; + if (hasProhibitedClauses) { + throw new Error("SELECT queries with FROM/WHERE/GROUP/HAVING/WINDOW/ORDER/LIMIT/OFFSET/FETCH/FOR cannot be converted to VALUES."); + } const valueMap = new Map<string, ValueComponent>(); for (const item of query.selectClause.items) { const identifier = item.identifier?.name ?? null; if (!identifier) { throw new Error("Each SELECT item must have an alias matching target columns."); } - if (!valueMap.has(identifier)) { - valueMap.set(identifier, item.value); - } + if (valueMap.has(identifier)) { + throw new Error(`Duplicate SELECT item alias '${identifier}'.`); + } + valueMap.set(identifier, item.value); } + if (valueMap.size !== columnNames.length) { + throw new Error("SELECT item aliases must match the target column list 1:1."); + }
17-24: Clarify error when selectQuery is null vs not a VALUES queryIf
insertQuery.selectQueryis null, the current message (“not a VALUES query”) can mislead. Consider a more precise error for null/undefined.- if (!(valuesQuery instanceof ValuesQuery)) { - throw new Error("InsertQuery selectQuery is not a VALUES query."); - } + if (!valuesQuery) { + throw new Error("InsertQuery has no selectQuery to convert."); + } + if (!(valuesQuery instanceof ValuesQuery)) { + throw new Error("InsertQuery.selectQuery is not a VALUES query."); + }packages/core/src/transformers/QueryBuilder.ts (3)
40-46: Normalize after building the full binary tree (CTE hoisting completeness).normalize is called before appending later queries, so CTEs introduced by subsequent queries may be missed. Normalize once after the loop.
Apply this diff:
- CTENormalizer.normalize(result); @@ - for (let i = 2; i < queries.length; i++) { + for (let i = 2; i < queries.length; i++) { result.appendSelectQuery(operator, wrap(queries[i])); } - return result; + // Normalize after the full tree is built (captures CTEs from all branches). + result = CTENormalizer.normalize(result) as BinarySelectQuery; + return result;
253-255: Guard against alias collision (target vs source) and clarify the error message.If sourceAlias equals target alias/name, generated predicates and column refs become ambiguous. Also, the Update error message mentions alias even though a table name fallback is accepted.
Apply this diff:
@@ - if (!targetAlias) { - throw new Error(`Source expression does not have an alias. Please provide an alias for the source expression.`); - } + if (!targetAlias) { + throw new Error(`Unable to resolve target alias/name from source expression.`); + } + if (targetAlias === options.sourceAlias) { + throw new Error(`sourceAlias ('${options.sourceAlias}') must differ from target alias/name ('${targetAlias}').`); + } @@ - if (!targetAlias) { - throw new Error(`Source expression does not have an alias. Please provide an alias for the delete target.`); - } + if (!targetAlias) { + throw new Error(`Source expression must have an explicit alias for DELETE USING.`); + } + if (targetAlias === normalized.sourceAlias) { + throw new Error(`sourceAlias ('${normalized.sourceAlias}') must differ from target alias ('${targetAlias}').`); + } @@ - if (!targetAlias) { - throw new Error(`Source expression does not have an alias. Please provide an alias for the merge target.`); - } + if (!targetAlias) { + throw new Error(`Source expression must have an explicit alias for MERGE.`); + } + if (targetAlias === normalized.sourceAlias) { + throw new Error(`sourceAlias ('${normalized.sourceAlias}') must differ from target alias ('${targetAlias}').`); + }Also applies to: 287-289, 321-323
682-684: Be explicit about CTEDisabler side‑effect.Assign the returned query to make the mutation explicit and future‑proof if CTEDisabler switches to immutable behavior.
Apply this diff:
- const cteDisabler = new CTEDisabler(); - cteDisabler.execute(selectQuery); + const cteDisabler = new CTEDisabler(); + selectQuery = cteDisabler.execute(selectQuery) as SimpleSelectQuery;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/core/src/models/BinarySelectQuery.ts(2 hunks)packages/core/src/models/SelectQuery.ts(3 hunks)packages/core/src/models/SimpleSelectQuery.ts(2 hunks)packages/core/src/models/ValuesQuery.ts(2 hunks)packages/core/src/transformers/InsertQuerySelectValuesConverter.ts(1 hunks)packages/core/src/transformers/QueryBuilder.ts(3 hunks)packages/core/tests/transformers/InsertQuerySelectValuesConverter.test.ts(1 hunks)packages/core/tests/transformers/buildDeleteQuery.test.ts(1 hunks)packages/core/tests/transformers/buildInsertQuery.test.ts(1 hunks)packages/core/tests/transformers/buildMergeQuery.test.ts(1 hunks)packages/core/tests/transformers/buildUpdateQuery.test.ts(4 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/buildMergeQuery.test.tspackages/core/tests/transformers/InsertQuerySelectValuesConverter.test.tspackages/core/tests/transformers/buildInsertQuery.test.tspackages/core/src/transformers/InsertQuerySelectValuesConverter.tspackages/core/tests/transformers/buildUpdateQuery.test.tspackages/core/src/models/SelectQuery.tspackages/core/src/models/BinarySelectQuery.tspackages/core/src/models/SimpleSelectQuery.tspackages/core/tests/transformers/buildDeleteQuery.test.tspackages/core/src/transformers/QueryBuilder.tspackages/core/src/models/ValuesQuery.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/buildMergeQuery.test.tspackages/core/tests/transformers/InsertQuerySelectValuesConverter.test.tspackages/core/tests/transformers/buildInsertQuery.test.tspackages/core/tests/transformers/buildUpdateQuery.test.tspackages/core/tests/transformers/buildDeleteQuery.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/buildMergeQuery.test.tspackages/core/tests/transformers/InsertQuerySelectValuesConverter.test.tspackages/core/tests/transformers/buildInsertQuery.test.tspackages/core/src/transformers/InsertQuerySelectValuesConverter.tspackages/core/tests/transformers/buildUpdateQuery.test.tspackages/core/src/models/SelectQuery.tspackages/core/src/models/BinarySelectQuery.tspackages/core/src/models/SimpleSelectQuery.tspackages/core/tests/transformers/buildDeleteQuery.test.tspackages/core/src/transformers/QueryBuilder.tspackages/core/src/models/ValuesQuery.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/buildMergeQuery.test.tspackages/core/tests/transformers/InsertQuerySelectValuesConverter.test.tspackages/core/tests/transformers/buildInsertQuery.test.tspackages/core/tests/transformers/buildUpdateQuery.test.tspackages/core/tests/transformers/buildDeleteQuery.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/buildInsertQuery.test.tspackages/core/tests/transformers/buildUpdateQuery.test.ts
🧬 Code graph analysis (11)
packages/core/tests/transformers/buildMergeQuery.test.ts (5)
packages/core/src/parsers/SelectQueryParser.ts (1)
SelectQueryParser(31-669)packages/core/src/models/SimpleSelectQuery.ts (1)
SimpleSelectQuery(44-733)packages/core/src/transformers/QueryBuilder.ts (1)
QueryBuilder(19-720)packages/core/src/models/MergeQuery.ts (4)
MergeUpdateAction(11-21)MergeInsertAction(33-51)MergeDoNothingAction(53-55)MergeDeleteAction(23-31)packages/core/src/models/ValueComponent.ts (2)
ColumnReference(47-82)ValueList(38-45)
packages/core/tests/transformers/InsertQuerySelectValuesConverter.test.ts (2)
packages/core/src/parsers/InsertQueryParser.ts (1)
InsertQueryParser(14-106)packages/core/src/transformers/InsertQuerySelectValuesConverter.ts (1)
InsertQuerySelectValuesConverter(12-125)
packages/core/tests/transformers/buildInsertQuery.test.ts (3)
packages/core/src/parsers/SelectQueryParser.ts (1)
SelectQueryParser(31-669)packages/core/src/models/SimpleSelectQuery.ts (1)
SimpleSelectQuery(44-733)packages/core/src/transformers/QueryBuilder.ts (1)
QueryBuilder(19-720)
packages/core/src/transformers/InsertQuerySelectValuesConverter.ts (5)
packages/core/src/models/ValuesQuery.ts (1)
ValuesQuery(23-70)packages/core/src/models/SimpleSelectQuery.ts (1)
SimpleSelectQuery(44-733)packages/core/src/models/Clause.ts (3)
SelectItem(6-15)SelectClause(17-28)identifier(170-172)packages/core/src/models/BinarySelectQuery.ts (1)
BinarySelectQuery(36-216)packages/core/src/models/ValueComponent.ts (2)
ValueComponent(5-27)TupleExpression(396-403)
packages/core/tests/transformers/buildUpdateQuery.test.ts (2)
packages/core/src/parsers/SelectQueryParser.ts (1)
SelectQueryParser(31-669)packages/core/src/transformers/QueryBuilder.ts (1)
QueryBuilder(19-720)
packages/core/src/models/SelectQuery.ts (2)
packages/core/src/models/ValueComponent.ts (3)
name(120-122)name(383-385)SqlParameterValue(223-223)packages/core/src/models/SimpleSelectQuery.ts (1)
SimpleSelectQuery(44-733)
packages/core/src/models/BinarySelectQuery.ts (1)
packages/core/src/models/SelectQuery.ts (9)
BinarySelectQuery(100-100)InsertQueryConversionOptions(43-46)InsertQuery(102-102)UpdateQueryConversionOptions(48-53)UpdateQuery(103-103)DeleteQueryConversionOptions(55-60)DeleteQuery(104-104)MergeQueryConversionOptions(66-75)MergeQuery(105-105)
packages/core/src/models/SimpleSelectQuery.ts (2)
packages/core/src/models/SelectQuery.ts (9)
SelectQuery(88-97)InsertQueryConversionOptions(43-46)InsertQuery(102-102)UpdateQueryConversionOptions(48-53)UpdateQuery(103-103)DeleteQueryConversionOptions(55-60)DeleteQuery(104-104)MergeQueryConversionOptions(66-75)MergeQuery(105-105)packages/core/src/transformers/QueryBuilder.ts (1)
QueryBuilder(19-720)
packages/core/tests/transformers/buildDeleteQuery.test.ts (2)
packages/core/src/models/SimpleSelectQuery.ts (1)
SimpleSelectQuery(44-733)packages/core/src/transformers/QueryBuilder.ts (1)
QueryBuilder(19-720)
packages/core/src/transformers/QueryBuilder.ts (9)
packages/core/src/models/SelectQuery.ts (9)
SimpleSelectQuery(99-99)InsertQueryConversionOptions(43-46)InsertQuery(102-102)UpdateQueryConversionOptions(48-53)UpdateQuery(103-103)DeleteQueryConversionOptions(55-60)DeleteQuery(104-104)MergeQueryConversionOptions(66-75)MergeQuery(105-105)packages/core/src/models/SimpleSelectQuery.ts (1)
SimpleSelectQuery(44-733)packages/core/src/parsers/SourceExpressionParser.ts (1)
SourceExpressionParser(7-72)packages/core/src/models/Clause.ts (10)
InsertClause(576-589)UpdateClause(541-557)SetClauseItem(497-539)SetClause(476-483)FromClause(306-336)WhereClause(49-56)DeleteClause(559-569)UsingClause(338-348)SelectItem(6-15)WithClause(370-381)packages/core/src/models/MergeQuery.ts (5)
MergeWhenClause(57-69)MergeUpdateAction(11-21)MergeDeleteAction(23-31)MergeDoNothingAction(53-55)MergeInsertAction(33-51)packages/core/src/models/ValueComponent.ts (3)
ValueList(38-45)ColumnReference(47-82)BinaryExpression(198-209)packages/core/src/transformers/SelectValueCollector.ts (1)
SelectValueCollector(13-269)packages/core/src/transformers/CTECollector.ts (1)
CTECollector(25-501)packages/core/src/transformers/CTEDisabler.ts (1)
CTEDisabler(25-443)
packages/core/src/models/ValuesQuery.ts (2)
packages/core/src/models/SelectQuery.ts (9)
SimpleSelectQuery(99-99)InsertQueryConversionOptions(43-46)InsertQuery(102-102)UpdateQueryConversionOptions(48-53)UpdateQuery(103-103)DeleteQueryConversionOptions(55-60)DeleteQuery(104-104)MergeQueryConversionOptions(66-75)MergeQuery(105-105)packages/core/src/transformers/QueryBuilder.ts (1)
QueryBuilder(19-720)
🔇 Additional comments (11)
packages/core/tests/transformers/buildInsertQuery.test.ts (1)
8-20: LGTM: solid coverage for INSERT conversions and CTE hoisting.
- CTE hoist + options path, inferred columns, optional column dropping, explicit list filtering, and no-overlap error are all asserted precisely with SqlFormatter output. Good use of exact-equality for formatter-stable strings.
Also applies to: 22-29, 31-41, 43-53, 55-62
packages/core/tests/transformers/buildDeleteQuery.test.ts (1)
8-19: LGTM: DELETE with PKs and additional match columns is correctly asserted.USING source aliasing and equality predicates look right; strings match formatter output.
Also applies to: 21-33
packages/core/tests/transformers/buildUpdateQuery.test.ts (1)
8-19: LGTM: comprehensive UPDATE coverage (CTE, aliasing, composite PKs, column selection, errors).Legacy signature, options path, and error cases are well exercised with precise SqlFormatter assertions.
Also applies to: 21-33, 35-45, 47-58, 60-75, 77-89, 91-103, 105-114, 116-125
packages/core/tests/transformers/buildMergeQuery.test.ts (1)
9-41: LGTM: MERGE actions and column planning validated at AST level with good breadth.Aliases, action kinds, update/set items, insert columns/values, and error paths are covered.
Also applies to: 43-64, 66-82, 84-100, 102-111, 113-124
packages/core/tests/transformers/InsertQuerySelectValuesConverter.test.ts (1)
18-26: LGTM: converter round‑trip and error coverage look solid with normalized SQL checks.Factory helper keeps the formatter usage tidy; assertions are precise.
Also applies to: 28-36, 38-47, 49-54, 56-61
packages/core/src/models/SimpleSelectQuery.ts (1)
129-131: UNION ALL helper looks goodConsistent with existing toBinaryQuery delegation. No issues.
packages/core/src/models/ValuesQuery.ts (1)
41-59: Delegations LGTMUsing toSimpleQuery() as a funnel point is clean and consistent.
packages/core/src/models/BinarySelectQuery.ts (2)
168-171: exceptAllRaw helper LGTMMatches the pattern used by other raw helpers.
173-187: New conversion helpers LGTMDelegation to toSimpleQuery() keeps behavior consistent across query variants.
packages/core/src/models/SelectQuery.ts (1)
88-97: Interface extension LGTMUnified toXQuery surface across query variants is clear and consistent.
packages/core/src/transformers/InsertQuerySelectValuesConverter.ts (1)
52-57: No issues found—code uses correct constructor keyThe verification confirms the constructor accepts
returningas the parameter key (line 24 of InsertQuery.ts), and the code correctly passesreturning: insertQuery.returningClause. The field assignmentthis.returningClause = params.returning ?? null;(line 30) handles the mapping internally.
| private static resolveInsertColumns(selectQuery: SimpleSelectQuery, options: InsertQueryConversionOptions): string[] { | ||
| const selectItemCount = selectQuery.selectClause.items.length; | ||
| if (options.columns && options.columns.length > 0) { | ||
| if (options.columns.length !== selectItemCount) { | ||
| throw new Error(`The number of provided columns (${options.columns.length}) must match the select list size (${selectItemCount}).`); | ||
| } | ||
| return [...options.columns]; | ||
| } | ||
|
|
||
| const items = QueryBuilder.collectSelectItems(selectQuery); | ||
| const inferred = items.map(item => item.name).filter(name => name !== '*'); | ||
| if (!inferred.length || inferred.length !== selectItemCount) { | ||
| throw new Error( | ||
| `Columns cannot be inferred from the selectQuery. Make sure you are not using wildcards or unnamed columns.\n` + | ||
| `Select clause column count: ${selectItemCount}, ` + | ||
| `Columns with valid names: ${inferred.length}\n` + | ||
| `Detected column names: [${inferred.join(", ")}]` | ||
| ); | ||
| } | ||
| return inferred; | ||
| } | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Unused helper: resolveInsertColumns — remove or wire up.
This private method is not referenced. Keep code lean; delete it or adopt it in buildInsertQuery.
Run to confirm it’s unused:
🏁 Script executed:
#!/bin/bash
rg -nP '\bresolveInsertColumns\s*\(' -g 'packages/**' -SLength of output: 173
I need to verify more thoroughly by checking the buildInsertQuery method and the immediate context around this method definition.The web search results are for generic query builders and ORMs, not specific to this codebase. Let me run a more targeted search within the repository to examine the code context and verify the claim.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the buildInsertQuery method and surrounding context in QueryBuilder.ts
rg -nA 20 'buildInsertQuery' packages/core/src/transformers/QueryBuilder.ts -C 5
Length of output: 826
---
🏁 Script executed:
```shell
#!/bin/bash
# Find prepareInsertColumns and see if it calls resolveInsertColumns
rg -nA 30 'private static prepareInsertColumns' packages/core/src/transformers/QueryBuilder.ts
Length of output: 2000
---
🏁 Script executed:
```shell
#!/bin/bash
# Final confirmation: check for any string references to the method name
rg -n 'resolveInsertColumns' packages/core/src/transformers/QueryBuilder.ts
Length of output: 126
Remove unused method resolveInsertColumns.
This private static method (lines 395–416) has no call sites within the codebase. The buildInsertQuery flow uses prepareInsertColumns instead. Delete the method to maintain code cleanliness.
🤖 Prompt for AI Agents
packages/core/src/transformers/QueryBuilder.ts around lines 395 to 416: the
private static method resolveInsertColumns is unused and should be removed;
delete the entire method declaration (including its signature and body) and any
now-unused imports or helper references, run a repo-wide search to confirm there
are no remaining call sites, and run tests/tsc to ensure no compile or runtime
regressions.
| private static prepareInsertColumns(selectQuery: SimpleSelectQuery, optionColumns: string[] | null): string[] { | ||
| const selectColumns = QueryBuilder.collectSelectColumnNames(selectQuery); | ||
| if (optionColumns && optionColumns.length > 0) { | ||
| const normalized = QueryBuilder.normalizeColumnArray(optionColumns); | ||
| const allowed = new Set(normalized); | ||
| const finalColumns = selectColumns.filter(name => allowed.has(name)); | ||
| if (!finalColumns.length) { | ||
| throw new Error('No overlapping columns found between selectQuery and provided columns.'); | ||
| } | ||
| QueryBuilder.rebuildSelectClause(selectQuery, finalColumns); | ||
| QueryBuilder.ensureSelectClauseSize(selectQuery, finalColumns.length); | ||
| return finalColumns; | ||
| } |
There was a problem hiding this comment.
Explicit columns are treated as a filter, not as the target order (INSERT).
columns should define the final target order, and the SELECT must be reordered to match it. Current code keeps select-order and ignores caller-provided ordering, which is surprising and can produce mismatched intent.
Apply this diff to preserve caller order and validate missing columns:
@@
- if (optionColumns && optionColumns.length > 0) {
- const normalized = QueryBuilder.normalizeColumnArray(optionColumns);
- const allowed = new Set(normalized);
- const finalColumns = selectColumns.filter(name => allowed.has(name));
- if (!finalColumns.length) {
- throw new Error('No overlapping columns found between selectQuery and provided columns.');
- }
- QueryBuilder.rebuildSelectClause(selectQuery, finalColumns);
- QueryBuilder.ensureSelectClauseSize(selectQuery, finalColumns.length);
- return finalColumns;
- }
+ if (optionColumns && optionColumns.length > 0) {
+ const normalized = QueryBuilder.normalizeColumnArray(optionColumns);
+ // Preserve caller order and ensure all requested columns exist in the select list.
+ const ordered = normalized.filter((name) => selectColumns.includes(name));
+ const missing = normalized.filter((name) => !selectColumns.includes(name));
+ if (missing.length) {
+ throw new Error(`Provided insert columns not found in selectQuery: [${missing.join(', ')}].`);
+ }
+ QueryBuilder.rebuildSelectClause(selectQuery, ordered);
+ QueryBuilder.ensureSelectClauseSize(selectQuery, ordered.length);
+ return ordered;
+ }🤖 Prompt for AI Agents
In packages/core/src/transformers/QueryBuilder.ts around lines 499 to 511, the
current logic treats provided optionColumns as a filter but preserves the
SELECT's original order; change it so optionColumns define the final INSERT
order: normalize optionColumns, compute which of those exist in selectColumns,
if any requested columns are missing throw a clear error listing the missing
names, set finalColumns to the optionColumns order filtered to existing ones,
then call QueryBuilder.rebuildSelectClause(selectQuery, finalColumns) and
QueryBuilder.ensureSelectClauseSize(selectQuery, finalColumns.length) so the
SELECT is reordered to match the caller-provided column order.
| private static prepareMergeColumns( | ||
| selectQuery: SimpleSelectQuery, | ||
| primaryKeys: string[], | ||
| explicitUpdateColumns: string[] | null, | ||
| explicitInsertColumns: string[] | null, | ||
| matchedAction: string, | ||
| notMatchedAction: string | ||
| ): { updateColumns: string[]; insertColumns: string[] } { | ||
| const selectColumns = QueryBuilder.collectSelectColumnNames(selectQuery); | ||
| QueryBuilder.ensurePrimaryKeys(selectColumns, primaryKeys); | ||
|
|
||
| const primaryKeySet = new Set(primaryKeys); | ||
|
|
||
| let updateColumnsSeed: string[] = []; | ||
| if (matchedAction === 'update') { | ||
| const candidates = selectColumns.filter(name => !primaryKeySet.has(name)); | ||
| if (explicitUpdateColumns && explicitUpdateColumns.length > 0) { | ||
| const normalized = QueryBuilder.normalizeColumnArray(explicitUpdateColumns); | ||
| const preferred = new Set(normalized); | ||
| updateColumnsSeed = candidates.filter(name => preferred.has(name)); | ||
| } else { | ||
| updateColumnsSeed = candidates; | ||
| } | ||
| } | ||
|
|
||
| let insertColumnsSeed: string[] = []; | ||
| if (notMatchedAction === 'insert') { | ||
| if (explicitInsertColumns && explicitInsertColumns.length > 0) { | ||
| const normalized = QueryBuilder.normalizeColumnArray(explicitInsertColumns); | ||
| const preferred = new Set(normalized); | ||
| insertColumnsSeed = selectColumns.filter(name => preferred.has(name)); | ||
| } else { | ||
| insertColumnsSeed = selectColumns; | ||
| } | ||
| } | ||
|
|
||
| const requiredColumns = new Set<string>(); | ||
| primaryKeys.forEach(key => requiredColumns.add(key)); | ||
| updateColumnsSeed.forEach(col => requiredColumns.add(col)); | ||
| insertColumnsSeed.forEach(col => requiredColumns.add(col)); | ||
|
|
||
| const desiredOrder = selectColumns.filter(name => requiredColumns.has(name)); | ||
| QueryBuilder.rebuildSelectClause(selectQuery, desiredOrder); | ||
| QueryBuilder.ensureSelectClauseSize(selectQuery, desiredOrder.length); | ||
|
|
||
| const finalUpdateColumns = matchedAction === 'update' | ||
| ? desiredOrder.filter(name => !primaryKeySet.has(name) && updateColumnsSeed.includes(name)) | ||
| : []; | ||
| const finalInsertColumns = notMatchedAction === 'insert' | ||
| ? desiredOrder.filter(name => insertColumnsSeed.includes(name)) | ||
| : []; | ||
|
|
||
| return { | ||
| updateColumns: finalUpdateColumns, | ||
| insertColumns: finalInsertColumns | ||
| }; | ||
| } |
There was a problem hiding this comment.
MERGE insert/update columns should preserve explicit order and drive SELECT reordering.
Like INSERT, explicit updateColumns/insertColumns must dictate both action column order and SELECT value order. Current implementation filters by presence and keeps select-order, ignoring caller-provided ordering.
Apply this diff to honor explicit ordering and validate missing columns:
@@
- let updateColumnsSeed: string[] = [];
- if (matchedAction === 'update') {
- const candidates = selectColumns.filter(name => !primaryKeySet.has(name));
- if (explicitUpdateColumns && explicitUpdateColumns.length > 0) {
- const normalized = QueryBuilder.normalizeColumnArray(explicitUpdateColumns);
- const preferred = new Set(normalized);
- updateColumnsSeed = candidates.filter(name => preferred.has(name));
- } else {
- updateColumnsSeed = candidates;
- }
- }
+ let updateColumnsSeed: string[] = [];
+ if (matchedAction === 'update') {
+ const candidates = selectColumns.filter(name => !primaryKeySet.has(name));
+ if (explicitUpdateColumns && explicitUpdateColumns.length > 0) {
+ const normalized = QueryBuilder.normalizeColumnArray(explicitUpdateColumns);
+ const ordered = normalized.filter((name) => candidates.includes(name));
+ const missing = normalized.filter((name) => !candidates.includes(name));
+ if (missing.length) {
+ throw new Error(`Provided update columns not found in selectQuery or are primary keys: [${missing.join(', ')}].`);
+ }
+ updateColumnsSeed = ordered;
+ } else {
+ updateColumnsSeed = candidates;
+ }
+ }
@@
- let insertColumnsSeed: string[] = [];
- if (notMatchedAction === 'insert') {
- if (explicitInsertColumns && explicitInsertColumns.length > 0) {
- const normalized = QueryBuilder.normalizeColumnArray(explicitInsertColumns);
- const preferred = new Set(normalized);
- insertColumnsSeed = selectColumns.filter(name => preferred.has(name));
- } else {
- insertColumnsSeed = selectColumns;
- }
- }
+ let insertColumnsSeed: string[] = [];
+ if (notMatchedAction === 'insert') {
+ if (explicitInsertColumns && explicitInsertColumns.length > 0) {
+ const normalized = QueryBuilder.normalizeColumnArray(explicitInsertColumns);
+ const ordered = normalized.filter((name) => selectColumns.includes(name));
+ const missing = normalized.filter((name) => !selectColumns.includes(name));
+ if (missing.length) {
+ throw new Error(`Provided insert columns not found in selectQuery: [${missing.join(', ')}].`);
+ }
+ insertColumnsSeed = ordered;
+ } else {
+ insertColumnsSeed = selectColumns;
+ }
+ }
@@
- const requiredColumns = new Set<string>();
- primaryKeys.forEach(key => requiredColumns.add(key));
- updateColumnsSeed.forEach(col => requiredColumns.add(col));
- insertColumnsSeed.forEach(col => requiredColumns.add(col));
-
- const desiredOrder = selectColumns.filter(name => requiredColumns.has(name));
+ // Desired order: PKs first, then ordered update columns, then ordered insert columns.
+ const desiredOrder = Array.from(new Set([
+ ...primaryKeys,
+ ...updateColumnsSeed,
+ ...insertColumnsSeed
+ ])).filter((name) => selectColumns.includes(name));🤖 Prompt for AI Agents
In packages/core/src/transformers/QueryBuilder.ts around lines 567 to 623, the
merge preparation currently derives update/insert column order from the SELECT
clause instead of preserving caller-provided explicit ordering and doesn't
validate missing explicit columns; change it so explicit
updateColumns/insertColumns (when provided) dictate the action column order and
drive the SELECT reordering: normalize the explicit arrays preserving their
order, check each explicit name exists in selectColumns and throw a clear error
listing any missing names, use the explicit-ordered lists as the seeds (instead
of filtering selectColumns by presence) and build desiredOrder by combining
primaryKeys (in given order), then explicit updateColumns (if
matchedAction==='update'), then explicit insertColumns (if
notMatchedAction==='insert') while avoiding duplicates, finally append any
remaining selectColumns in their original order, call rebuildSelectClause with
that desiredOrder and compute finalUpdateColumns/finalInsertColumns from the
explicit-ordered seeds (or from the candidate lists when explicit lists are not
provided).
1 participant
Summary by CodeRabbit
New Features