Skip to content

Commit 439c406

Browse files
authored
fix(core): deduplicate moved conditions (#948)
1 parent 992fa01 commit 439c406

5 files changed

Lines changed: 198 additions & 0 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"rawsql-ts": patch
3+
---
4+
5+
Deduplicate identical top-level AND conditions when condition optimization moves a predicate into a WHERE or JOIN ON clause that already contains the same predicate.

packages/core/src/transformers/ParameterConditionPlacementOptimizer.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ import {
3737
SqlComponentFormatOptions
3838
} from "./SqlComponentFormatter";
3939
import { SelectOutputCollector, SelectOutputColumn } from "./SelectOutputCollector";
40+
import {
41+
dedupeTopLevelAndConditions,
42+
dedupeWhereTopLevelAndConditions
43+
} from "./TopLevelAndConditionDeduper";
4044

4145
export type ParameterConditionOptimizationInput = string | SelectQuery | SimpleSelectQuery;
4246

@@ -335,6 +339,7 @@ export class ParameterConditionPlacementOptimizer {
335339

336340
const movedCondition = this.rebaseCondition(candidate.expression, targetColumns, options);
337341
target.query.appendWhere(movedCondition);
342+
dedupeWhereTopLevelAndConditions(target.query, options);
338343
appliedReason = placement.reason;
339344
} else {
340345
const placements: TargetPlacement[] = [];
@@ -355,6 +360,7 @@ export class ParameterConditionPlacementOptimizer {
355360
for (const branch of target.branches) {
356361
const movedCondition = this.rebaseCondition(candidate.expression, branch.targetColumns, options);
357362
branch.query.appendWhere(movedCondition);
363+
dedupeWhereTopLevelAndConditions(branch.query, options);
358364
}
359365
appliedReason = placements.some(item => /group by/i.test(item.reason))
360366
? "Condition is distributed to every UNION branch by output column position; grouped branches only receive GROUP BY-key predicates."
@@ -1031,6 +1037,7 @@ export class ParameterConditionPlacementOptimizer {
10311037
"and",
10321038
cloneValueComponent(expression, options)
10331039
);
1040+
join.condition.condition = dedupeTopLevelAndConditions(join.condition.condition, options);
10341041
}
10351042

10361043
private resolveTargetColumnByOutputIndex(

packages/core/src/transformers/StaticPredicatePlacementOptimizer.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ import {
3737
SqlComponentFormatOptions
3838
} from "./SqlComponentFormatter";
3939
import { SelectOutputCollector, SelectOutputColumn } from "./SelectOutputCollector";
40+
import {
41+
dedupeTopLevelAndConditions,
42+
dedupeWhereTopLevelAndConditions
43+
} from "./TopLevelAndConditionDeduper";
4044

4145
export type StaticPredicatePlacementInput = string | SelectQuery | SimpleSelectQuery;
4246

@@ -331,6 +335,7 @@ export class StaticPredicatePlacementOptimizer {
331335

332336
const movedPredicate = this.rebasePredicate(candidate.expression, targetColumns, options);
333337
target.query.appendWhere(movedPredicate);
338+
dedupeWhereTopLevelAndConditions(target.query, options);
334339
appliedReason = placement.reason;
335340
} else {
336341
const placements: TargetPlacement[] = [];
@@ -351,6 +356,7 @@ export class StaticPredicatePlacementOptimizer {
351356
for (const branch of target.branches) {
352357
const movedPredicate = this.rebasePredicate(candidate.expression, branch.targetColumns, options);
353358
branch.query.appendWhere(movedPredicate);
359+
dedupeWhereTopLevelAndConditions(branch.query, options);
354360
}
355361
appliedReason = placements.some(item => /group by/i.test(item.reason))
356362
? "Predicate is distributed to every UNION branch by output column position; grouped branches only receive GROUP BY-key predicates."
@@ -1027,6 +1033,7 @@ export class StaticPredicatePlacementOptimizer {
10271033
"and",
10281034
cloneValueComponent(expression, options)
10291035
);
1036+
join.condition.condition = dedupeTopLevelAndConditions(join.condition.condition, options);
10301037
}
10311038

10321039
private resolveDeepestBranchTarget(
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
import { WhereClause } from "../models/Clause";
2+
import { SimpleSelectQuery } from "../models/SelectQuery";
3+
import {
4+
BinaryExpression,
5+
ParenExpression,
6+
ValueComponent
7+
} from "../models/ValueComponent";
8+
import { formatSqlComponent, SqlComponentFormatOptions } from "./SqlComponentFormatter";
9+
10+
// API output shape review: this internal helper keeps optimizer result sql/query shape unchanged;
11+
// formatSqlComponent is used only as a caller-configurable canonical key for exact duplicate detection.
12+
const unwrapParens = (expression: ValueComponent): ValueComponent => {
13+
let candidate = expression;
14+
while (candidate instanceof ParenExpression) {
15+
candidate = candidate.expression;
16+
}
17+
return candidate;
18+
};
19+
20+
const isAndExpression = (expression: ValueComponent): expression is BinaryExpression => {
21+
const candidate = unwrapParens(expression);
22+
return candidate instanceof BinaryExpression
23+
&& candidate.operator.value.trim().toLowerCase() === "and";
24+
};
25+
26+
const collectTopLevelAndTerms = (expression: ValueComponent): ValueComponent[] => {
27+
const candidate = unwrapParens(expression);
28+
if (!isAndExpression(candidate)) {
29+
return [expression];
30+
}
31+
32+
return [
33+
...collectTopLevelAndTerms(candidate.left),
34+
...collectTopLevelAndTerms(candidate.right)
35+
];
36+
};
37+
38+
export const dedupeTopLevelAndConditions = (
39+
expression: ValueComponent,
40+
options: SqlComponentFormatOptions
41+
): ValueComponent => {
42+
const terms = collectTopLevelAndTerms(expression);
43+
if (terms.length < 2) {
44+
return expression;
45+
}
46+
47+
const seen = new Set<string>();
48+
const uniqueTerms: ValueComponent[] = [];
49+
for (const term of terms) {
50+
const key = formatSqlComponent(unwrapParens(term), options);
51+
if (seen.has(key)) {
52+
continue;
53+
}
54+
seen.add(key);
55+
uniqueTerms.push(term);
56+
}
57+
58+
if (uniqueTerms.length === terms.length) {
59+
return expression;
60+
}
61+
62+
let rebuilt = uniqueTerms[0]!;
63+
for (let index = 1; index < uniqueTerms.length; index += 1) {
64+
rebuilt = new BinaryExpression(rebuilt, "and", uniqueTerms[index]!);
65+
}
66+
return rebuilt;
67+
};
68+
69+
export const dedupeWhereTopLevelAndConditions = (
70+
query: SimpleSelectQuery,
71+
options: SqlComponentFormatOptions
72+
): void => {
73+
if (!query.whereClause) {
74+
return;
75+
}
76+
77+
query.whereClause = new WhereClause(dedupeTopLevelAndConditions(query.whereClause.condition, options));
78+
};

packages/core/tests/transformers/ConditionOptimization.test.ts

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,72 @@ describe('ConditionOptimization', () => {
486486
`));
487487
});
488488

489+
it('deduplicates identical parameter conditions after moving into an existing upstream WHERE', () => {
490+
const sql = `
491+
with active_users as (
492+
select u.id, u.status
493+
from users u
494+
where u.status = :status
495+
)
496+
select *
497+
from active_users au
498+
where au.status = :status
499+
`;
500+
501+
const result = optimizeConditions(sql);
502+
503+
expect(result.ok).toBe(true);
504+
expect(result.applied).toEqual([
505+
expect.objectContaining({
506+
phaseKind: 'parameter_condition_placement',
507+
kind: 'move_condition',
508+
toScopeId: 'cte:active_users',
509+
columnReferences: ['au.status']
510+
})
511+
]);
512+
expect(result.skipped).toEqual([]);
513+
expect(normalizeSql(result.sql)).toBe(normalizeSql(`
514+
with active_users as (
515+
select u.id, u.status
516+
from users u
517+
where u.status = :status
518+
)
519+
select *
520+
from active_users au
521+
`));
522+
});
523+
524+
it('deduplicates identical parameter conditions after moving into an existing JOIN ON condition', () => {
525+
const sql = `
526+
select *
527+
from orders o
528+
join users u
529+
on u.id = o.user_id
530+
and u.status = :status
531+
where u.status = :status
532+
`;
533+
534+
const result = optimizeConditions(sql);
535+
536+
expect(result.ok).toBe(true);
537+
expect(result.applied).toEqual([
538+
expect.objectContaining({
539+
phaseKind: 'parameter_condition_placement',
540+
kind: 'move_condition',
541+
toScopeId: 'join_on:u',
542+
columnReferences: ['u.status']
543+
})
544+
]);
545+
expect(result.skipped).toEqual([]);
546+
expect(normalizeSql(result.sql)).toBe(normalizeSql(`
547+
select *
548+
from orders o
549+
join users u
550+
on u.id = o.user_id
551+
and u.status = :status
552+
`));
553+
});
554+
489555
it('keeps ambiguous wildcard outputs skipped instead of guessing a source column', () => {
490556
const sql = `
491557
select *
@@ -843,6 +909,41 @@ describe('ConditionOptimization', () => {
843909
`));
844910
});
845911

912+
it('deduplicates identical static predicates after moving into an existing upstream WHERE', () => {
913+
const sql = `
914+
with active_users as (
915+
select u.id, u.status
916+
from users u
917+
where u.status = 'active'
918+
)
919+
select *
920+
from active_users au
921+
where au.status = 'active'
922+
`;
923+
924+
const result = optimizeConditions(sql);
925+
926+
expect(result.ok).toBe(true);
927+
expect(result.applied).toEqual([
928+
expect.objectContaining({
929+
phaseKind: 'static_predicate_placement',
930+
kind: 'move_static_predicate',
931+
toScopeId: 'cte:active_users',
932+
columnReferences: ['au.status']
933+
})
934+
]);
935+
expect(result.skipped).toEqual([]);
936+
expect(normalizeSql(result.sql)).toBe(normalizeSql(`
937+
with active_users as (
938+
select u.id, u.status
939+
from users u
940+
where u.status = 'active'
941+
)
942+
select *
943+
from active_users au
944+
`));
945+
});
946+
846947
it('prunes same-block optional WHERE branches even when the query groups later', () => {
847948
const sql = `
848949
select customer_id, count(*) as order_count

0 commit comments

Comments
 (0)