feat(core): add native UNION support via sequelize.union#18110
feat(core): add native UNION support via sequelize.union#18110agamennone-siae wants to merge 4 commits intosequelize:mainfrom
Conversation
…eries with support for `UNION ALL`, `ORDER BY`, `LIMIT`, and `OFFSET`.
📝 WalkthroughWalkthroughThis PR adds UNION query support across multiple Sequelize layers. A new Changes
Sequence DiagramsequenceDiagram
participant Client
participant Sequelize
participant QueryInterface
participant QueryGenerator
participant Database
Client->>Sequelize: union(queries, options)
activate Sequelize
Sequelize->>Sequelize: Normalize & prepare each query<br/>(inject scopes, includes, attributes)
Sequelize->>Sequelize: Handle deduplication<br/>(inject primary key if needed)
loop For each query
Sequelize->>QueryGenerator: buildSelectQuery(query)
QueryGenerator-->>Sequelize: SELECT SQL
end
Sequelize->>QueryInterface: union(sqls, options)
deactivate Sequelize
activate QueryInterface
QueryInterface->>QueryGenerator: unionQuery(sqls, options)
activate QueryGenerator
QueryGenerator->>QueryGenerator: Join SQLs with UNION/UNION ALL
QueryGenerator->>QueryGenerator: Append ORDER BY, LIMIT, OFFSET
QueryGenerator-->>QueryInterface: Combined SQL
deactivate QueryGenerator
QueryInterface->>Database: Execute combined SQL
activate Database
Database-->>QueryInterface: Result set
deactivate Database
QueryInterface-->>Client: Results
deactivate QueryInterface
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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: 8
🤖 Fix all issues with AI agents
In `@packages/core/src/abstract-dialect/query-generator.js`:
- Around line 1495-1506: The ORDER BY direction from options.order is used
directly when building orderClauses; validate the direction against
VALID_ORDER_OPTIONS before interpolating to avoid unexpected SQL. In the order
mapping logic (where orders, orderClauses, and the tuple [col, dir] are
handled), normalize dir to a canonical form (e.g., toUpperCase()), check
inclusion in VALID_ORDER_OPTIONS, and if invalid either throw a clear error or
fall back to a safe default like 'ASC'; then use the validated value when
calling quoteIdentifier(col) and building the clause. Ensure VALID_ORDER_OPTIONS
is referenced and used for validation so behavior matches the rest of the
codebase.
- Around line 1509-1514: The conditional that decides whether to call
this.#internals.addLimitAndOffset uses a truthy check (if (options.limit ||
options.offset)) — change it to use explicit null/undefined checks to match the
file's pattern by testing options.limit != null || options.offset != null so
LIMIT/OFFSET with zero values are handled consistently; update the conditional
near the query assembly where query and limitOrder are used (the block calling
this.#internals.addLimitAndOffset with options.limit/options.offset).
In `@packages/core/src/abstract-dialect/query-interface.js`:
- Around line 585-588: Before calling this.sequelize.queryRaw in the union flow,
remove options.replacements the same way select() does to avoid double
replacements: obtain or clone the options passed to unionQuery (referencing
this.queryGenerator.unionQuery and the local variable options), delete the
replacements property (options.replacements) on that cloned options object, then
pass the sanitized options into this.sequelize.queryRaw(sql, sanitizedOptions).
Ensure you do not mutate the original options object if other callers rely on
it.
- Around line 583-584: The merge currently allows callers to override the query
type; in the async union(sqls, options) function ensure QueryTypes.SELECT is
applied after merging so callers cannot override it. Modify the options
assignment to first merge user options (e.g., options = { ...options }) and then
enforce options.type = QueryTypes.SELECT (or recreate options = { ...options,
type: QueryTypes.SELECT }) so the function always uses SELECT; update references
to options in union accordingly.
In `@packages/core/src/sequelize.js`:
- Around line 247-252: In union, add upfront validation to ensure the queries
array is not empty and each entry has a valid model: check that queries is an
array with length > 0 and iterate queries to verify q && q.model is present (and
optionally that q.options exists); throw a clear TypeError or return a rejected
Promise with a descriptive message if validation fails. Place this check at the
start of the async union(queries, options = {}) method before mapping queries so
that later uses of q.model and q.options (e.g., the mapping that references
q.model and q.options) are safe and produce deterministic errors.
- Line 287: Update the UnionOptions type so it includes transaction support by
extending QueryRawOptions instead of only AddLimitOffsetOptions: find the
UnionOptions type (used by the union method) and change its extends clause to
include QueryRawOptions (which includes Transactionable) so callers can pass a
transaction through the union() call that ultimately delegates to queryRaw;
ensure any existing properties (limit, offset, replacements) remain present and
compatible with QueryRawOptions.
In `@packages/core/test/integration/sequelize/union.test.ts`:
- Line 41: The tests cast sequelize to any because the custom union method is
missing from the Sequelize type definitions; add a TypeScript declaration for
the union method (its signature used in tests) and surface it via module
augmentation or by extending the Sequelize interface so the test instance has
the correct type instead of casting. Concretely, add a declaration file (or
update the package's exported types) that augments the 'sequelize' module /
Sequelize class with union(params: Array<{ model: typeof Model }>):
Promise<any[]> (match the real return type), export that types file from the
package (or include it in the package's types entry), then remove the (sequelize
as any) casts in union.test.ts so the test uses the updated typed
sequelize.union method.
- Around line 96-109: Test currently asserts that 'age' is absent but doesn't
assert whether the primary key was injected when attributes were specified;
update the test for sequelize.union to also assert presence (or absence, as
appropriate) of the injected primary key field (id) when calling (sequelize as
any).union with options { attributes: ['name'] } and raw mode not set, e.g.,
after creating vars.User and vars.Guest check results include the primary key
property 'id' (or verify deduplication behavior relies on id) so the behavior of
attribute injection in the union implementation (union, attributes, raw, id,
vars.User, vars.Guest) is covered.
🧹 Nitpick comments (4)
packages/core/src/abstract-dialect/query-generator.types.ts (1)
15-18: Consider adding JSDoc documentation for the new public interface.The
UnionOptionsinterface is part of the public API but lacks documentation. Adding JSDoc comments would improve developer experience.📝 Suggested documentation
+/** + * Options for union queries. + */ export interface UnionOptions extends AddLimitOffsetOptions { + /** If true, use UNION ALL (keeps duplicates). Defaults to false (UNION with deduplication). */ unionAll?: boolean; + /** Order clause for the combined result. Can be a column name, [column, direction] tuple, or array of either. */ order?: string | [string, string] | Array<string | [string, string]>; }packages/core/src/sequelize.d.ts (1)
611-617: Consider using a more specific type for thequeriesparameter.The
queriesparameter is typed asobject[], which is quite loose. Based on the implementation insequelize.js, each query object should havemodeland optionaloptionsproperties.💡 Suggested type improvement
+export interface UnionQueryEntry { + model: ModelStatic<any>; + options?: FindOptions<any>; +} + /** * Run a union query * * `@param` queries The queries to union. Can be a set of options or model instances. * `@param` options The options for the union query. */ -union(queries: object[], options?: UnionOptions): Promise<any>; +union(queries: UnionQueryEntry[], options?: UnionOptions): Promise<any>;packages/core/test/integration/sequelize/union.test.ts (1)
5-5: Add dialect teaser for test description.Per coding guidelines, use
Support.getTestDialectTeaser()for dialect-specific test descriptions.📝 Suggested fix
+import { sequelize, getTestDialectTeaser } from '../support'; -import { sequelize } from '../support'; -describe('Sequelize#union', () => { +describe(getTestDialectTeaser('Sequelize#union'), () => {packages/core/src/sequelize.js (1)
264-274: Add comments explaining the primary key injection logic.The condition for primary key injection is complex. Adding a comment would help future maintainers understand when and why the primary key is injected.
📝 Suggested comment
+ // Inject primary key into attributes if: + // - Custom attributes are specified (not selecting all columns) + // - Not a raw query (raw queries bypass model mapping) + // - Model has a primary key + // - Primary key is not already in the attributes + // - Either no grouping, or has multi-association (to ensure proper deduplication) if ( queryOptions.attributes && !queryOptions.raw && model.primaryKeyAttribute && !queryOptions.attributes.includes(model.primaryKeyAttribute) && (!queryOptions.group || !queryOptions.hasSingleAssociation || queryOptions.hasMultiAssociation) ) { queryOptions.attributes = [model.primaryKeyAttribute].concat(queryOptions.attributes); }
| if (options.order) { | ||
| const orders = Array.isArray(options.order) ? options.order : [options.order]; | ||
| const orderClauses = orders.map(t => { | ||
| if (Array.isArray(t)) { | ||
| const [col, dir] = t; | ||
|
|
||
| return `${this.quoteIdentifier(col)} ${dir}`; | ||
| } | ||
|
|
||
| return this.quoteIdentifier(t); | ||
| }); | ||
| query += ` ORDER BY ${orderClauses.join(', ')}`; |
There was a problem hiding this comment.
Order direction should be validated to prevent unexpected SQL.
The dir value from the order tuple is interpolated directly into SQL without validation. While this may not be a direct SQL injection risk (as it's typically controlled by developers), validating against VALID_ORDER_OPTIONS would be consistent with the rest of the codebase.
🛡️ Suggested validation
const orderClauses = orders.map(t => {
if (Array.isArray(t)) {
const [col, dir] = t;
+ const upperDir = dir?.toUpperCase();
+ if (dir && !['ASC', 'DESC', 'ASC NULLS FIRST', 'ASC NULLS LAST', 'DESC NULLS FIRST', 'DESC NULLS LAST', 'NULLS FIRST', 'NULLS LAST'].includes(upperDir)) {
+ throw new Error(`Invalid order direction: ${dir}`);
+ }
- return `${this.quoteIdentifier(col)} ${dir}`;
+ return `${this.quoteIdentifier(col)}${dir ? ` ${upperDir}` : ''}`;
}
return this.quoteIdentifier(t);
});🤖 Prompt for AI Agents
In `@packages/core/src/abstract-dialect/query-generator.js` around lines 1495 -
1506, The ORDER BY direction from options.order is used directly when building
orderClauses; validate the direction against VALID_ORDER_OPTIONS before
interpolating to avoid unexpected SQL. In the order mapping logic (where orders,
orderClauses, and the tuple [col, dir] are handled), normalize dir to a
canonical form (e.g., toUpperCase()), check inclusion in VALID_ORDER_OPTIONS,
and if invalid either throw a clear error or fall back to a safe default like
'ASC'; then use the validated value when calling quoteIdentifier(col) and
building the clause. Ensure VALID_ORDER_OPTIONS is referenced and used for
validation so behavior matches the rest of the codebase.
| if (options.limit || options.offset) { | ||
| const limitOrder = this.#internals.addLimitAndOffset(options); | ||
| if (limitOrder) { | ||
| query += ` ${limitOrder}`; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find packages -name "query-generator.js" -o -name "query-generator.ts" | head -20Repository: sequelize/sequelize
Length of output: 467
🏁 Script executed:
cat -n packages/core/src/abstract-dialect/query-generator.js | sed -n '1500,1530p'Repository: sequelize/sequelize
Length of output: 981
🏁 Script executed:
ast-grep --pattern 'addLimitAndOffset($_) {
$$$
}'Repository: sequelize/sequelize
Length of output: 45
🏁 Script executed:
rg -A 5 "class.*extends.*QueryGenerator" packages/Repository: sequelize/sequelize
Length of output: 11640
🏁 Script executed:
rg -A 10 "addLimitAndOffset" packages/core/src/abstract-dialect/ -t jsRepository: sequelize/sequelize
Length of output: 1855
🏁 Script executed:
rg -A 10 "addLimitAndOffset" packages/ -t ts -t js | head -100Repository: sequelize/sequelize
Length of output: 7631
🏁 Script executed:
rg "limit.*offset|offset.*limit" packages/core/src/abstract-dialect/query-generator.js -B 3 -A 3Repository: sequelize/sequelize
Length of output: 2384
🏁 Script executed:
rg "offset.*0|limit.*10" packages/core/tests -A 3 -B 1 | head -60Repository: sequelize/sequelize
Length of output: 122
🏁 Script executed:
rg "limit.*!=|limit.*null" packages/core/src/abstract-dialect/query-generator.js | head -20Repository: sequelize/sequelize
Length of output: 549
Condition uses truthy/falsy check; align with codebase pattern using explicit null checks.
The specific example ({ limit: 10, offset: 0 }) will not skip the LIMIT clause—10 || 0 evaluates to 10 (truthy), so the condition passes. However, there is an inconsistency: this condition uses implicit truthiness evaluation while elsewhere in the same file it uses explicit checks like options.limit != null || options.offset (lines 1440, 1452, 1464). For consistency and clarity, consider aligning this condition to use options.limit != null || options.offset instead of relying on truthy/falsy behavior.
🤖 Prompt for AI Agents
In `@packages/core/src/abstract-dialect/query-generator.js` around lines 1509 -
1514, The conditional that decides whether to call
this.#internals.addLimitAndOffset uses a truthy check (if (options.limit ||
options.offset)) — change it to use explicit null/undefined checks to match the
file's pattern by testing options.limit != null || options.offset != null so
LIMIT/OFFSET with zero values are handled consistently; update the conditional
near the query assembly where query and limitOrder are used (the block calling
this.#internals.addLimitAndOffset with options.limit/options.offset).
| async union(sqls, options) { | ||
| options = { type: QueryTypes.SELECT, ...options }; |
There was a problem hiding this comment.
Force type to SELECT after merging options.
The current merge order lets callers override type, which can change result parsing vs select. Set type last to keep behavior consistent.
Proposed fix
- options = { type: QueryTypes.SELECT, ...options };
+ options = { ...options, type: QueryTypes.SELECT };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async union(sqls, options) { | |
| options = { type: QueryTypes.SELECT, ...options }; | |
| async union(sqls, options) { | |
| options = { ...options, type: QueryTypes.SELECT }; |
🤖 Prompt for AI Agents
In `@packages/core/src/abstract-dialect/query-interface.js` around lines 583 -
584, The merge currently allows callers to override the query type; in the async
union(sqls, options) function ensure QueryTypes.SELECT is applied after merging
so callers cannot override it. Modify the options assignment to first merge user
options (e.g., options = { ...options }) and then enforce options.type =
QueryTypes.SELECT (or recreate options = { ...options, type: QueryTypes.SELECT
}) so the function always uses SELECT; update references to options in union
accordingly.
| const sql = this.queryGenerator.unionQuery(sqls, options); | ||
|
|
||
| return await this.sequelize.queryRaw(sql, options); | ||
| } |
There was a problem hiding this comment.
Drop options.replacements before queryRaw, like select().
This keeps replacement handling consistent and avoids double/inconsistent replacement application.
Proposed fix
const sql = this.queryGenerator.unionQuery(sqls, options);
+ // unlike bind, replacements are handled by QueryGenerator, not QueryRaw
+ delete options.replacements;
return await this.sequelize.queryRaw(sql, options);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const sql = this.queryGenerator.unionQuery(sqls, options); | |
| return await this.sequelize.queryRaw(sql, options); | |
| } | |
| const sql = this.queryGenerator.unionQuery(sqls, options); | |
| // unlike bind, replacements are handled by QueryGenerator, not QueryRaw | |
| delete options.replacements; | |
| return await this.sequelize.queryRaw(sql, options); | |
| } |
🤖 Prompt for AI Agents
In `@packages/core/src/abstract-dialect/query-interface.js` around lines 585 -
588, Before calling this.sequelize.queryRaw in the union flow, remove
options.replacements the same way select() does to avoid double replacements:
obtain or clone the options passed to unionQuery (referencing
this.queryGenerator.unionQuery and the local variable options), delete the
replacements property (options.replacements) on that cloned options object, then
pass the sanitized options into this.sequelize.queryRaw(sql, sanitizedOptions).
Ensure you do not mutate the original options object if other callers rely on
it.
| async union(queries, options = {}) { | ||
| // Basic preparations similar to Model.findAll, ensuring options are normalized. | ||
| const rawSqls = await Promise.all( | ||
| queries.map(async q => { | ||
| const model = q.model; | ||
| const queryOptions = { ...q.options }; |
There was a problem hiding this comment.
Add validation for empty queries array and model presence.
The method doesn't validate that the queries array is non-empty or that each query has a valid model. This could lead to confusing errors downstream.
🛡️ Suggested validation
async union(queries, options = {}) {
+ if (!Array.isArray(queries) || queries.length === 0) {
+ throw new Error('union() requires at least one query');
+ }
+
// Basic preparations similar to Model.findAll, ensuring options are normalized.
const rawSqls = await Promise.all(
queries.map(async q => {
const model = q.model;
+ if (!model) {
+ throw new Error('Each union query must specify a model');
+ }
const queryOptions = { ...q.options };🤖 Prompt for AI Agents
In `@packages/core/src/sequelize.js` around lines 247 - 252, In union, add upfront
validation to ensure the queries array is not empty and each entry has a valid
model: check that queries is an array with length > 0 and iterate queries to
verify q && q.model is present (and optionally that q.options exists); throw a
clear TypeError or return a rejected Promise with a descriptive message if
validation fails. Place this check at the start of the async union(queries,
options = {}) method before mapping queries so that later uses of q.model and
q.options (e.g., the mapping that references q.model and q.options) are safe and
produce deterministic errors.
| }), | ||
| ); | ||
|
|
||
| return this.queryInterface.union(rawSqls, options); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if queryInterface.union supports transaction options
rg -A 20 "^ union\(" packages/core/src/abstract-dialect/query-interface.jsRepository: sequelize/sequelize
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Also check the TypeScript version
rg -A 20 "union\(" packages/core/src/abstract-dialect/query-interface.tsRepository: sequelize/sequelize
Length of output: 156
🏁 Script executed:
#!/bin/bash
# Check the sequelize.js file to see the full context of the union method
rg -B 5 -A 5 "union\(" packages/core/src/sequelize.jsRepository: sequelize/sequelize
Length of output: 752
🏁 Script executed:
#!/bin/bash
# Check how other query methods handle transactions
rg -A 10 "transaction" packages/core/src/abstract-dialect/query-interface.ts | head -50Repository: sequelize/sequelize
Length of output: 156
🏁 Script executed:
#!/bin/bash
# Find all query-interface files
fd "query-interface" packages/Repository: sequelize/sequelize
Length of output: 1690
🏁 Script executed:
#!/bin/bash
# Search for union method definition across the codebase
rg "union\(" packages/ --type js --type ts -B 2 -A 15 | head -100Repository: sequelize/sequelize
Length of output: 6272
🏁 Script executed:
#!/bin/bash
# Check the directory structure of abstract-dialect
ls -la packages/core/src/abstract-dialect/ | head -30Repository: sequelize/sequelize
Length of output: 1572
🏁 Script executed:
#!/bin/bash
# Check queryRaw implementation to see if it handles transaction options
rg -A 15 "async queryRaw" packages/core/src/abstract-dialect/query-interface.jsRepository: sequelize/sequelize
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check the TypeScript version of union method
rg -A 10 "union\(" packages/core/src/abstract-dialect/query-interface-typescript.tsRepository: sequelize/sequelize
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check if there are tests for union with transactions
rg -B 2 -A 10 "union.*transaction\|transaction.*union" packages/core/test/ --type js --type tsRepository: sequelize/sequelize
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check UnionOptions type to see what options are supported
rg -B 5 -A 10 "UnionOptions" packages/core/src/Repository: sequelize/sequelize
Length of output: 8725
🏁 Script executed:
#!/bin/bash
# Check AddLimitOffsetOptions to see what options are included
rg -B 2 -A 15 "interface AddLimitOffsetOptions" packages/core/src/Repository: sequelize/sequelize
Length of output: 1436
🏁 Script executed:
#!/bin/bash
# Check the sequelize.queryRaw method to see if it handles transaction
rg -B 5 -A 25 "async queryRaw" packages/core/src/sequelize.jsRepository: sequelize/sequelize
Length of output: 1185
🏁 Script executed:
#!/bin/bash
# Look for transaction option handling in query execution
rg "transaction" packages/core/src/abstract-dialect/query-interface.js -A 2 -B 2 | head -60Repository: sequelize/sequelize
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check how select or other methods handle transaction options
rg -B 5 -A 15 "transaction" packages/core/src/abstract-dialect/query-interface-typescript.ts | head -80Repository: sequelize/sequelize
Length of output: 2772
🏁 Script executed:
#!/bin/bash
# Look for base QueryOptions or similar that includes transaction
rg "interface.*Options" packages/core/src/abstract-dialect/query-interface.types.ts -A 8 | head -100Repository: sequelize/sequelize
Length of output: 4616
🏁 Script executed:
#!/bin/bash
# Check how queryRaw actually executes the query and if transaction is used
rg -A 40 "async queryRaw" packages/core/src/sequelize.js | tail -50Repository: sequelize/sequelize
Length of output: 1890
🏁 Script executed:
#!/bin/bash
# Find QueryRawOptions definition
rg -B 2 -A 20 "interface QueryRawOptions\|type QueryRawOptions" packages/core/src/Repository: sequelize/sequelize
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check if other union-like methods extend QueryRawOptions
rg -B 3 -A 10 "interface.*Options.*extends.*QueryRawOptions" packages/core/src/abstract-dialect/query-interface.types.ts | head -60Repository: sequelize/sequelize
Length of output: 2699
🏁 Script executed:
#!/bin/bash
# Search in sequelize.ts for QueryRawOptions
rg -B 5 -A 25 "interface QueryRawOptions\|type QueryRawOptions" packages/core/src/sequelize.tsRepository: sequelize/sequelize
Length of output: 133
🏁 Script executed:
#!/bin/bash
# Check how sequelize.query handles transactions vs sequelize.queryRaw
rg -B 5 -A 15 "^\s+query\(" packages/core/src/sequelize.js | head -50Repository: sequelize/sequelize
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Confirm UnionOptions is used in union method signature
cat packages/core/src/abstract-dialect/query-interface.d.ts | grep -A 5 "union("Repository: sequelize/sequelize
Length of output: 196
🏁 Script executed:
#!/bin/bash
# Search for QueryRawOptions in all files
rg "QueryRawOptions" packages/core/src/ -lRepository: sequelize/sequelize
Length of output: 445
🏁 Script executed:
#!/bin/bash
# Check the sequelize.d.ts for QueryRawOptions
rg -B 5 -A 20 "QueryRawOptions" packages/core/src/sequelize.d.ts | head -80Repository: sequelize/sequelize
Length of output: 2635
🏁 Script executed:
#!/bin/bash
# Check if there are any tests that show union with transaction working or failing
fd ".*test.*" packages/core/test/ -type f | xargs grep -l "union" | head -5Repository: sequelize/sequelize
Length of output: 233
🏁 Script executed:
#!/bin/bash
# Search for Transactionable interface
rg -B 2 -A 10 "interface Transactionable" packages/core/src/Repository: sequelize/sequelize
Length of output: 916
🏁 Script executed:
#!/bin/bash
# Check query tests to see if union with transaction is tested
find packages/core/test -name "*.js" -o -name "*.ts" | xargs grep -l "\.union" | head -3Repository: sequelize/sequelize
Length of output: 117
🏁 Script executed:
#!/bin/bash
# Read the union test file to see what's tested
cat packages/core/test/integration/sequelize/union.test.tsRepository: sequelize/sequelize
Length of output: 3296
🏁 Script executed:
#!/bin/bash
# Check how the union method is implemented in query-interface-typescript to be thorough
rg -B 5 -A 15 "union\(" packages/core/src/abstract-dialect/query-interface-typescript.tsRepository: sequelize/sequelize
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check if union is defined in query-interface-typescript at all
rg "union" packages/core/src/abstract-dialect/query-interface-typescript.tsRepository: sequelize/sequelize
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Let's verify the complete flow: sequelize.union -> queryInterface.union -> queryRaw
# Check the actual implementation in query-interface-typescript.ts to see union method
cat packages/core/src/abstract-dialect/query-interface-typescript.ts | grep -A 10 "union"Repository: sequelize/sequelize
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Let me verify the UnionOptions definition one more time and see what it really extends
cat packages/core/src/abstract-dialect/query-generator.types.ts | grep -B 2 -A 10 "export interface UnionOptions"Repository: sequelize/sequelize
Length of output: 476
🏁 Script executed:
#!/bin/bash
# Check how select or other QueryInterface methods define their options
rg "selectQuery\|findAll" packages/core/src/abstract-dialect/query-interface.d.ts -A 3 | head -30Repository: sequelize/sequelize
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check if there's any guidance in the codebase about why UnionOptions doesn't extend QueryRawOptions
git log --all --oneline --grep="UnionOptions\|union" -- packages/core/src/abstract-dialect/query-generator.types.ts 2>/dev/null | head -5Repository: sequelize/sequelize
Length of output: 89
🏁 Script executed:
#!/bin/bash
# Check the commit that added union to understand intent
git show 434ec30 --stat 2>/dev/null | head -20Repository: sequelize/sequelize
Length of output: 1099
🏁 Script executed:
#!/bin/bash
# Let me also verify what Logging and Poolable interfaces contain in QueryRawOptions
rg -B 2 -A 8 "interface Logging\|interface Poolable" packages/core/src/model.d.tsRepository: sequelize/sequelize
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Let me check what the actual transaction behavior would be
# by looking at how other methods handle transactions with QueryRawOptions
rg -B 3 -A 10 "interface.*Options extends.*QueryRawOptions" packages/core/src/abstract-dialect/query-interface.types.ts | grep -A 10 "Select\|Find" | head -30Repository: sequelize/sequelize
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Final check: confirm UnionOptions is the intended type for union method
rg "union.*UnionOptions" packages/core/src/Repository: sequelize/sequelize
Length of output: 398
Extend UnionOptions to include transaction support via QueryRawOptions.
The union method's UnionOptions type definition only extends AddLimitOffsetOptions (limit, offset, replacements), but the underlying queryRaw call accepts QueryRawOptions, which includes Transactionable (transaction property). Update UnionOptions to extend QueryRawOptions so users can pass transaction options without TypeScript type violations. Currently, the type signature prevents valid transactions even though the runtime implementation would support them.
🤖 Prompt for AI Agents
In `@packages/core/src/sequelize.js` at line 287, Update the UnionOptions type so
it includes transaction support by extending QueryRawOptions instead of only
AddLimitOffsetOptions: find the UnionOptions type (used by the union method) and
change its extends clause to include QueryRawOptions (which includes
Transactionable) so callers can pass a transaction through the union() call that
ultimately delegates to queryRaw; ensure any existing properties (limit, offset,
replacements) remain present and compatible with QueryRawOptions.
| await vars.User.create({ name: 'Alice', age: 20 }); | ||
| await vars.Guest.create({ name: 'Bob', age: 30 }); | ||
|
|
||
| const results = await (sequelize as any).union([{ model: vars.User }, { model: vars.Guest }]); |
There was a problem hiding this comment.
Type cast indicates missing type definition.
The (sequelize as any).union cast is used throughout the tests, indicating that the union method type isn't properly exported or the test's sequelize instance doesn't have the updated types. This should be addressed to ensure type safety in tests.
🤖 Prompt for AI Agents
In `@packages/core/test/integration/sequelize/union.test.ts` at line 41, The tests
cast sequelize to any because the custom union method is missing from the
Sequelize type definitions; add a TypeScript declaration for the union method
(its signature used in tests) and surface it via module augmentation or by
extending the Sequelize interface so the test instance has the correct type
instead of casting. Concretely, add a declaration file (or update the package's
exported types) that augments the 'sequelize' module / Sequelize class with
union(params: Array<{ model: typeof Model }>): Promise<any[]> (match the real
return type), export that types file from the package (or include it in the
package's types entry), then remove the (sequelize as any) casts in
union.test.ts so the test uses the updated typed sequelize.union method.
| it('allows specifying attributes in query options', async () => { | ||
| await vars.User.create({ name: 'Alice', age: 20 }); | ||
| await vars.Guest.create({ name: 'Bob', age: 30 }); | ||
|
|
||
| // Only select name | ||
| const results = await (sequelize as any).union([ | ||
| { model: vars.User, options: { attributes: ['name'] } }, | ||
| { model: vars.Guest, options: { attributes: ['name'] } }, | ||
| ]); | ||
|
|
||
| expect(results).to.have.lengthOf(2); | ||
| expect(results[0]).to.have.property('name'); | ||
| expect(results[0]).to.not.have.property('age'); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find packages/core/src -name "*union*" -o -name "*sequelize*" | head -20Repository: sequelize/sequelize
Length of output: 296
🏁 Script executed:
rg -l "union" packages/core/src --type ts --type js | head -10Repository: sequelize/sequelize
Length of output: 433
🏁 Script executed:
rg -A 30 "^\s*(union|\.union)\s*\(" packages/core/src/sequelize.js | head -100Repository: sequelize/sequelize
Length of output: 45
🏁 Script executed:
wc -l packages/core/src/sequelize.jsRepository: sequelize/sequelize
Length of output: 97
🏁 Script executed:
rg "union" packages/core/src/sequelize.js -A 20Repository: sequelize/sequelize
Length of output: 1605
🏁 Script executed:
cat packages/core/test/integration/sequelize/union.test.ts | head -120Repository: sequelize/sequelize
Length of output: 3296
🏁 Script executed:
rg "Inject primary key if needed for deduplication" packages/core/src/sequelize.js -A 40Repository: sequelize/sequelize
Length of output: 1564
Test incompletely verifies attribute filtering—does not check for injected primary key.
The union implementation injects the primary key when attributes is specified and !raw is true. In this test, since attributes: ['name'] is specified and the primary key is not included, the implementation will inject the id column into the query. The test verifies that age is absent but does not verify whether id appears in the results, leaving a gap in coverage of the deduplication behavior that relies on the primary key.
🤖 Prompt for AI Agents
In `@packages/core/test/integration/sequelize/union.test.ts` around lines 96 -
109, Test currently asserts that 'age' is absent but doesn't assert whether the
primary key was injected when attributes were specified; update the test for
sequelize.union to also assert presence (or absence, as appropriate) of the
injected primary key field (id) when calling (sequelize as any).union with
options { attributes: ['name'] } and raw mode not set, e.g., after creating
vars.User and vars.Guest check results include the primary key property 'id' (or
verify deduplication behavior relies on id) so the behavior of attribute
injection in the union implementation (union, attributes, raw, id, vars.User,
vars.Guest) is covered.
1 participant
Pull Request Checklist
Description of Changes
Implemented
Sequelize#union(feat #18107) to combine results from multiple queries.This adds support for:
UNION(default) andUNION ALL(via{ unionAll: true }option).ORDER BYclause for the combined result.LIMITandOFFSETclauses.The implementation relies on a new
unionQuerymethod inAbstractQueryGeneratorwhich handles the SQL generation across dialects, properly quoting identifiers and handling dialect-specific syntax (e.g.LIMIT/OFFSEThandling).List of Breaking Changes
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.