Skip to content

Fix OR handling in inToMarkJoin and scalarToSingleJoin (#271) #322

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zhouqingqing merged 1 commit into from
Apr 14, 2026
Merged

Fix OR handling in inToMarkJoin and scalarToSingleJoin (#271) #322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 36 additions & 23 deletions qpmodel/subquery.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,16 +273,16 @@
// LogicNode_A
// LogicNode_B
//
LogicNode scalarToSingleJoin(LogicNode planWithSubExpr, ScalarSubqueryExpr scalarExpr)
LogicNode scalarToSingleJoin(LogicNode planWithSubExpr, ScalarSubqueryExpr scalarExpr, ref bool canReplace)
{
var newplan = planWithSubExpr;

// nodeB contains the join filter
var nodeSubquery = scalarExpr.query_.logicPlan_;

// make a single join
// nodeA
// <Subquery>
// nodeA
// <Subquery>
// nodeSubquery
// =>
// SingleJoin
Expand All @@ -302,7 +302,7 @@
newplan = djoinOnRightAggregation(singleJoinNode, scalarExpr);
break;
case LogicFilter lf:
newplan = djoinOnRightFilter(singleJoinNode, scalarExpr);
newplan = djoinOnRightFilter(singleJoinNode, scalarExpr, ref canReplace);
break;
default:
break;
Expand All @@ -320,20 +320,26 @@
return newplan;
}

Expr extractCurINExprFromNodeAFilter(LogicNode nodeA, InSubqueryExpr curInExpr, ExprRef markerFilter)
Expr extractCurINExprFromNodeAFilter(LogicNode nodeA, InSubqueryExpr curInExpr, ExprRef markerFilter, ref bool canReplace)
{
var nodeAFilter = nodeA.filter_;
if (nodeAFilter != null)
{
// a1 > @1 and a2 > @2 and a3 > 2, scalarExpr = @1
// keeplist: a1 > @1 and a3 > 2
// andlist after removal: a2 > @2
// nodeAFilter = a1 > @1 and a3 > 2
// OR-aware logic (same principle as existsToMarkJoin):
// For IN subqueries, the InSubqueryExpr appears as a direct conjunct
// (e.g., WHERE a1 IN (select ...)), so Equals works for non-OR.
// For OR conjuncts, only remove if no extra subqueries in the OR.
//
var andlist = nodeAFilter.FilterToAndList();
var keeplist = andlist.Where(x => x.VisitEachExists(e => e.Equals(curInExpr))).ToList();
andlist.RemoveAll(x => x.VisitEachExists(e => e.Equals(curInExpr)));
if (andlist.Count == 0)
andlist.RemoveAll(x =>
(!(x is LogicOrExpr) && x.VisitEachExists(e => e.Equals(curInExpr))) ||
((x is LogicOrExpr) && !hasAnyExtraSubqueryExprInOR(x, curInExpr)));

// if there is any (#marker@1 or @2), the root should be replaced
canReplace = andlist.Find(x => (x is LogicOrExpr) && hasAnyExtraSubqueryExprInOR(x, curInExpr)) != null;

if (andlist.Count == 0 || canReplace)
nodeA.NullifyFilter();
else
{
Expand All @@ -347,7 +353,7 @@
return nodeAFilter;
}

LogicNode inToMarkJoin(LogicNode planWithSubExpr, InSubqueryExpr inExpr)
LogicNode inToMarkJoin(LogicNode planWithSubExpr, InSubqueryExpr inExpr, ref bool canReplace)
{
LogicNode nodeA = planWithSubExpr;

Expand All @@ -359,7 +365,7 @@
// nullify nodeA's filter: the rest is push to top filter. However,
// if nodeA is a Filter|MarkJoin, keep its mark filter.
var markerFilter = new ExprRef(new MarkerExpr(nodeBFilter.tableRefs_, inExpr.subqueryid_), 0);
var nodeAFilter = extractCurINExprFromNodeAFilter(nodeA, inExpr, markerFilter);
var nodeAFilter = extractCurINExprFromNodeAFilter(nodeA, inExpr, markerFilter, ref canReplace);

// consider SQL ...a1 in select b1 from...
// a1 is outerExpr and b1 is selectExpr
Expand Down Expand Up @@ -403,7 +409,7 @@
}

// D Xs (Filter(T)) => Filter(D Xs T)
LogicNode djoinOnRightFilter(LogicSingleJoin singleJoinNode, ScalarSubqueryExpr scalarExpr)
LogicNode djoinOnRightFilter(LogicSingleJoin singleJoinNode, ScalarSubqueryExpr scalarExpr, ref bool canReplace)
{
var nodeLeft = singleJoinNode.lchild_();
var nodeSubquery = singleJoinNode.rchild_();
Expand All @@ -419,15 +425,21 @@
var nodeLeftFilter = nodeLeft.filter_;
if (nodeLeftFilter != null)
{
// a1 > @1 and a2 > @2 and a3 > 2, scalarExpr = @1
// keeplist: a1 > @1 and a3 > 2
// andlist after removal: a2 > @2
// nodeAFilter = a1 > @1 and a3 > 2
// OR-aware logic (same principle as existsToMarkJoin):
// For scalar subqueries, conjuncts CONTAIN the scalarExpr (e.g.,
// a1 = @scalar), so use VisitEachExists for non-OR conjuncts.
// For OR conjuncts, only remove if no extra subqueries in the OR.
//
var andlist = nodeLeftFilter.FilterToAndList();
var keeplist = andlist.Where(x => x.VisitEachExists(e => e.Equals(scalarExpr))).ToList();
andlist.RemoveAll(x => x.VisitEachExists(e => e.Equals(scalarExpr)));
if (andlist.Count == 0)
andlist.RemoveAll(x =>
(!(x is LogicOrExpr) && x.VisitEachExists(e => e.Equals(scalarExpr))) ||
((x is LogicOrExpr) && !hasAnyExtraSubqueryExprInOR(x, scalarExpr)));

// if there is any (expr with @1 or @2), the root should be replaced
canReplace = andlist.Find(x => (x is LogicOrExpr) && hasAnyExtraSubqueryExprInOR(x, scalarExpr)) != null;

if (andlist.Count == 0 || canReplace)
nodeLeft.NullifyFilter();
else
Comment on lines +439 to 444
{
Expand Down Expand Up @@ -522,7 +534,8 @@
singleJoinNode.children_[1] = filterNode;

// now we have convert it to a right filter plan
var newplan = djoinOnRightFilter(singleJoinNode, scalarExpr);
bool unusedCanReplace = false;
var newplan = djoinOnRightFilter(singleJoinNode, scalarExpr, ref unusedCanReplace);
Comment on lines +537 to +538

@zhouqingqing zhouqingqing Apr 14, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good observation. For the aggregation path, the scalar subquery is wrapped in an aggregate (e.g., SELECT max(b.i) FROM b WHERE ...), which guarantees a single row result. The OR-in-outer-filter scenario is handled by scalarToSingleJoin before dispatching to djoinOnRightAggregation, so canReplace is already set correctly at that level. The unusedCanReplace in djoinOnRightAggregation is intentional — the inner call only restructures the aggregation plan.

Comment on lines +537 to +538
return newplan;
}

Expand Down Expand Up @@ -1082,10 +1095,10 @@
newplan = existsToMarkJoin(planWithSubExpr, se, ref canRepalce);
break;
case ScalarSubqueryExpr ss:
newplan = scalarToSingleJoin(planWithSubExpr, ss);
newplan = scalarToSingleJoin(planWithSubExpr, ss, ref canRepalce);
break;
case InSubqueryExpr si:
newplan = inToMarkJoin(planWithSubExpr, si);
newplan = inToMarkJoin(planWithSubExpr, si, ref canRepalce);
break;
default:
break;
Expand Down Expand Up @@ -1490,7 +1503,7 @@
{
internal CteExpr cte_;

public CteInfoEntry cteInfoEntry_;

Check warning on line 1506 in qpmodel/subquery.cs

View workflow job for this annotation

GitHub Actions / build 

Field 'LogicCteProducer.cte_' is never assigned to, and will always have its default value null

Check warning on line 1506 in qpmodel/subquery.cs

View workflow job for this annotation

GitHub Actions / build 

Field 'LogicCteProducer.cte_' is never assigned to, and will always have its default value null

Check warning on line 1506 in qpmodel/subquery.cs

View workflow job for this annotation

GitHub Actions / build 

Field 'LogicCteProducer.cte_' is never assigned to, and will always have its default value null

Check warning on line 1506 in qpmodel/subquery.cs

View workflow job for this annotation

GitHub Actions / build 

Field 'LogicCteProducer.cte_' is never assigned to, and will always have its default value null
// represent the id of CTE, it should match the related CteProducer
public int cteId_;
public override string ToString() => $"CteProducer({child_()})";
Expand Down
44 changes: 44 additions & 0 deletions test/regress/expect/subqueryd_or.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
select a1 from a where a2 in (select b2 from b where b2 = a1) or a2 > 2
PhysicFilter (actual rows=1)
Output: a.a1[0]
Filter: ({#marker@1}[1] or a.a2[2]>2)
-> PhysicMarkJoin Left (actual rows=3)
Output: a.a1[0],{#marker@1}[2],a.a2[1]
Filter: (a.a2[1]=b.b2[2] and b.b2[2]=a.a1[0])
-> PhysicScanTable a (actual rows=3)
Output: a.a1[0],a.a2[1]
-> PhysicScanTable b (actual rows=3, loops=3)
Output: b.b2[1]
2

select a1 from a where a2 not in (select b2 from b where b2 = a1) or a2 > 2
PhysicFilter (actual rows=3)
Output: a.a1[0]
Filter: ({#marker@1}[1] or a.a2[2]>2)
-> PhysicMarkJoin Left (actual rows=3)
Output: a.a1[0],{#marker@1}[2],a.a2[1]
Filter: (a.a2[1]=b.b2[2] and b.b2[2]=a.a1[0])
-> PhysicScanTable a (actual rows=3)
Output: a.a1[0],a.a2[1]
-> PhysicScanTable b (actual rows=3, loops=3)
Output: b.b2[1]
0
1
2

select a1, a3 from a where a.a1 = (select b1 from b where b2 = a2 and b3<4) or a2>1
PhysicFilter (actual rows=3)
Output: a.a1[0],a.a3[1]
Filter: (a.a1[0]=b.b1[2] or a.a2[3]>1)
-> PhysicSingleJoin Left (actual rows=3)
Output: a.a1[0],a.a3[1],b.b1[3],a.a2[2]
Filter: b.b2[4]=a.a2[2]
-> PhysicScanTable a (actual rows=3)
Output: a.a1[0],a.a3[2],a.a2[1]
-> PhysicScanTable b (actual rows=2, loops=3)
Output: b.b1[0],b.b2[1]
Filter: b.b3[2]<4
0,2
1,3
2,4

13 changes: 13 additions & 0 deletions test/regress/sql/subqueryd_or.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
-- Issue #271: OR handling in inToMarkJoin and scalarToSingleJoin
set enable_subquery_unnest = true;
set enable_dependent_join_pushdown = false;
set enable_neumann_full_decorrelation = false;

-- correlated IN subquery in OR: IN matches nothing, a2>2 matches a1=2
select a1 from a where a2 in (select b2 from b where b2 = a1) or a2 > 2;

-- NOT IN with OR
select a1 from a where a2 not in (select b2 from b where b2 = a1) or a2 > 2;

-- scalar subquery in OR
select a1, a3 from a where a.a1 = (select b1 from b where b2 = a2 and b3<4) or a2>1;
Loading