assertion failure with unique index + partitioning + join

Alexander Kuzmenkov <akuzmenkov@tigerdata.com> 于2026年6月15日周一 23:39写道：

Hi hackers,

While I was running some random testing on the TimescaleDB extension, I noticed that I can easily hit an assertion failure in plain Postgres REL_18_STABLE. I haven't analyzed it at all, but thought I should report this:

CREATE TABLE t (a bool UNIQUE) PARTITION BY LIST (a);
CREATE TABLE p PARTITION OF t DEFAULT;
SELECT
FROM (
SELECT *
FROM t
LEFT JOIN t r USING (a)
)
WHERE a
GROUP BY ();

TRAP: failed Assert("bms_is_member(i, root->outer_join_rels)"), File: "../pg/src/backend/optimizer/path/equivclass.c", Line: 3631, PID: 1813021

I can reproduce this issue on HEAD.
The root->outer_join_rels is empty, and the i is 3(rel: t r).
The left join can be removed in remove_useless_joins(), in which
root->outer_join_rels is changed to be empty.
But the from->qual in this case has already been distributed to its rel.
(gdb) pgprint brel->baserestrictinfo
RestrictInfo [is_pushed_down=true can_join=false pseudoconstant=false
has_clone=false is_clone=false leakproof=false
has_volatile=VOLATILITY_UNKNOWN security_level=0
num_base_rels=1 rinfo_serial=1 eval_cost={startup = -1,
per_tuple = 0} norm_selec=-1 outer_selec=-1 outer_is_left=false
hashjoinoperator=0 left_bucketsize=-1
right_bucketsize=-1 left_mcvfreq=-1 right_mcvfreq=-1
left_hasheqoperator=0 right_hasheqoperator=0]
[clause]
PlaceHolderVar [phid=1 phlevelsup=0]
[phexpr] Var [varno=2 varattno=1 vartype=16
varreturningtype=VAR_RETURNING_DEFAULT varnosyn=2 varattnosyn=1]
[phrels] Bitmapset [4 3 2]
[clause_relids] Bitmapset [2]
[required_relids] Bitmapset [2]

It was distributed to (rel:2, table t), but I think "[phrels]
Bitmapset [4 3 2]" is incorrect. After leftjoin is removed, we no
longer need rel=4 (outerjoin) and rel=3 (t r).
But the qual has already been distributed to the (rel: 2) when
deconstruct_jointree (). Then the qual is inherited by its child(table
p)
It seems that we have no logic to process this distributed
restrictinfo in remove_useless_joins().

I need to dig deeper to find out how to fix it.

--
Thanks,
Tender Wang

Hi all,

Tender Wang <tndrwang@gmail.com> 于2026年6月16日周二 09:58写道：

I can reproduce this issue on HEAD.
The root->outer_join_rels is empty, and the i is 3(rel: t r).
The left join can be removed in remove_useless_joins(), in which
root->outer_join_rels is changed to be empty.
But the from->qual in this case has already been distributed to its rel.
(gdb) pgprint brel->baserestrictinfo
RestrictInfo [is_pushed_down=true can_join=false pseudoconstant=false
has_clone=false is_clone=false leakproof=false
has_volatile=VOLATILITY_UNKNOWN security_level=0
num_base_rels=1 rinfo_serial=1 eval_cost={startup = -1,
per_tuple = 0} norm_selec=-1 outer_selec=-1 outer_is_left=false
hashjoinoperator=0 left_bucketsize=-1
right_bucketsize=-1 left_mcvfreq=-1 right_mcvfreq=-1
left_hasheqoperator=0 right_hasheqoperator=0]
[clause]
PlaceHolderVar [phid=1 phlevelsup=0]
[phexpr] Var [varno=2 varattno=1 vartype=16
varreturningtype=VAR_RETURNING_DEFAULT varnosyn=2 varattnosyn=1]
[phrels] Bitmapset [4 3 2]
[clause_relids] Bitmapset [2]
[required_relids] Bitmapset [2]
I need to dig deeper to find out how to fix it.

I tried on the old version (pg1212), I got the correct plan, no crash.
And after pulling up the subquery, no PlaceHolderVar. I found after
commit cc5d98525d43,

commit cc5d98525d43c22b98f360ef0f2c8d7dc57f04dc
Author: Richard Guo <rguo@postgresql.org>
Date: Thu Mar 13 16:36:03 2025 +0900

Fix incorrect handling of subquery pullup

the from->qual will include PHV, because this query contains groupingsets.
I reverted this commit, I got correct plan, too:
postgres=# explain SELECT
FROM (
SELECT *
FROM t
LEFT JOIN t r USING (a)
) ss
WHERE a
GROUP BY ();
QUERY PLAN
------------------------------------------
Result (cost=0.00..0.01 rows=1 width=0)
(1 row)

I added Richard to the cc list. He may know more about this.

--
Thanks,
Tender Wang

Tender Wang <tndrwang@gmail.com> 于2026年6月16日周二 12:06写道：

Hi all,

Tender Wang <tndrwang@gmail.com> 于2026年6月16日周二 09:58写道：

I can reproduce this issue on HEAD.
The root->outer_join_rels is empty, and the i is 3(rel: t r).
The left join can be removed in remove_useless_joins(), in which
root->outer_join_rels is changed to be empty.
But the from->qual in this case has already been distributed to its rel.
(gdb) pgprint brel->baserestrictinfo
RestrictInfo [is_pushed_down=true can_join=false pseudoconstant=false
has_clone=false is_clone=false leakproof=false
has_volatile=VOLATILITY_UNKNOWN security_level=0
num_base_rels=1 rinfo_serial=1 eval_cost={startup = -1,
per_tuple = 0} norm_selec=-1 outer_selec=-1 outer_is_left=false
hashjoinoperator=0 left_bucketsize=-1
right_bucketsize=-1 left_mcvfreq=-1 right_mcvfreq=-1
left_hasheqoperator=0 right_hasheqoperator=0]
[clause]
PlaceHolderVar [phid=1 phlevelsup=0]
[phexpr] Var [varno=2 varattno=1 vartype=16
varreturningtype=VAR_RETURNING_DEFAULT varnosyn=2 varattnosyn=1]
[phrels] Bitmapset [4 3 2]
[clause_relids] Bitmapset [2]
[required_relids] Bitmapset [2]
I need to dig deeper to find out how to fix it.

I tried on the old version (pg1212), I got the correct plan, no crash.
And after pulling up the subquery, no PlaceHolderVar. I found after
commit cc5d98525d43,

commit cc5d98525d43c22b98f360ef0f2c8d7dc57f04dc
Author: Richard Guo <rguo@postgresql.org>
Date: Thu Mar 13 16:36:03 2025 +0900

Fix incorrect handling of subquery pullup

the from->qual will include PHV, because this query contains groupingsets.
I reverted this commit, I got correct plan, too:
postgres=# explain SELECT
FROM (
SELECT *
FROM t
LEFT JOIN t r USING (a)
) ss
WHERE a
GROUP BY ();
QUERY PLAN
------------------------------------------
Result (cost=0.00..0.01 rows=1 width=0)
(1 row)

I added Richard to the cc list. He may know more about this.

In apply_child_basequals(), the restricintinfo->clause_relids of the
child is different from the parent's.
The parent's restricintinfo->clause_relids only contains rel:2, but
the child contains (3,4,5).
The difference is due to pull_varnos_walker() when processing the
PlaceholdVar case.

I do a quick fix as below:
diff --git a/src/backend/optimizer/util/inherit.c
b/src/backend/optimizer/util/inherit.c
index 6a7b9edff3f..fcf0ad7a187 100644
--- a/src/backend/optimizer/util/inherit.c
+++ b/src/backend/optimizer/util/inherit.c
@@ -861,6 +861,21 @@ apply_child_basequals(PlannerInfo *root,
RelOptInfo *parentrel,
                ListCell   *lc2;

                Assert(IsA(rinfo, RestrictInfo));
+               if (IsA(rinfo->clause, PlaceHolderVar))
+               {
+                       PlaceHolderVar *phv = (PlaceHolderVar *) rinfo->clause;
+                       PlaceHolderInfo *phinfo = NULL;
+                       if (phv->phlevelsup == 0)
+                       {
+                               if (phv->phid < root->placeholder_array_size)
+                                       phinfo =
root->placeholder_array[phv->phid];
+                       }
+                       if (phinfo != NULL)
+                       {
+                               phv->phrels = bms_copy(phinfo->ph_var->phrels);
+                       }
+
+               }

The issue has gone, and regression tests all pass.
But this is very simply fixed.
What do you think?

--
Thanks,
Tender Wang

On Tue, Jun 16, 2026 at 10:58 AM Tender Wang <tndrwang@gmail.com> wrote:

It was distributed to (rel:2, table t), but I think "[phrels]
Bitmapset [4 3 2]" is incorrect. After leftjoin is removed, we no
longer need rel=4 (outerjoin) and rel=3 (t r).
But the qual has already been distributed to the (rel: 2) when
deconstruct_jointree (). Then the qual is inherited by its child(table
p)
It seems that we have no logic to process this distributed
restrictinfo in remove_useless_joins().

When removing the left join, remove_rel_from_query() fixes up the
relid sets of RestrictInfos and EquivalenceMembers, and the canonical
PlaceHolderVars, but it does not rewrite the PlaceHolderVars embedded
in clause and EC-member expressions. This is usually fine, because
later processing consults those relid sets rather than the embedded
PHVs.

The trouble is that such an expression can later be translated for an
appendrel child and have its relids recomputed by pull_varnos(). If
the embedded PlaceHolderVar's phrels still mentions the removed
relation, pull_varnos() folds it back in, so the rebuilt clause
references a no-longer-existent rel. That produces a parameterized
path keyed on the removed relation, which trips the assertion.

I think we can fix it by stripping the removed relids from the PHVs in
surviving rels' baserestrictinfo and in EquivalenceClass member
expressions. See attached.

(I went back and forth on instead making pull_varnos() robust to the
stale phrels, but that turned out fragile: it can't reliably
distinguish a removed relid from a not-yet-built appendrel child, and
it broke unrelated partitionwise-join plans.)

Thoughts?

- Richard

Richard Guo <guofenglinux@gmail.com> 于2026年6月16日周二 16:27写道：

On Tue, Jun 16, 2026 at 10:58 AM Tender Wang <tndrwang@gmail.com> wrote:

It was distributed to (rel:2, table t), but I think "[phrels]
Bitmapset [4 3 2]" is incorrect. After leftjoin is removed, we no
longer need rel=4 (outerjoin) and rel=3 (t r).
But the qual has already been distributed to the (rel: 2) when
deconstruct_jointree (). Then the qual is inherited by its child(table
p)
It seems that we have no logic to process this distributed
restrictinfo in remove_useless_joins().

When removing the left join, remove_rel_from_query() fixes up the
relid sets of RestrictInfos and EquivalenceMembers, and the canonical
PlaceHolderVars, but it does not rewrite the PlaceHolderVars embedded
in clause and EC-member expressions. This is usually fine, because
later processing consults those relid sets rather than the embedded
PHVs.

The trouble is that such an expression can later be translated for an
appendrel child and have its relids recomputed by pull_varnos(). If
the embedded PlaceHolderVar's phrels still mentions the removed
relation, pull_varnos() folds it back in, so the rebuilt clause
references a no-longer-existent rel. That produces a parameterized
path keyed on the removed relation, which trips the assertion.

Yes, I came to the same conclusion.

I think we can fix it by stripping the removed relids from the PHVs in
surviving rels' baserestrictinfo and in EquivalenceClass member
expressions. See attached.

The fix I proposed was somewhat specific to this particular issue,
whereas your patch takes a more general approach.
I took a quick pass over the patch and didn't notice any obvious issues.
I won't have time to review it in detail today, but I'll try to take a
closer look tomorrow morning.

(I went back and forth on instead making pull_varnos() robust to the
stale phrels, but that turned out fragile: it can't reliably
distinguish a removed relid from a not-yet-built appendrel child, and
it broke unrelated partitionwise-join plans.)

Interesting, I hadn't thought of that approach.

--
Thanks,
Tender Wang

Hi all,

Richard Guo <guofenglinux@gmail.com> 于2026年6月16日周二 16:27写道：

I think we can fix it by stripping the removed relids from the PHVs in
surviving rels' baserestrictinfo and in EquivalenceClass member
expressions. See attached.

I spent some time reviewing the patch in detail.
A couple of minor suggestions:
In remove_rel_from_eclass(), we have
...
if (!IsA(em->em_expr, Var))
em->em_expr = (Expr *)
remove_rel_from_phvs((Node *) em->em_expr, relid, ojrelid);
...
"where s.id = 1" in the test case, the Const node seems to be able to skip, too.

Other than the minor suggestions above, the patch looks good to me.

I have another question, although it's unrelated to this bug.
The logic in remove_rel_from_query() makes me a bit uneasy. It
currently has to handle both self-join and outer-join cases,
and it seems possible that inner-join-specific handling could end up
being added there in the future as well.
The function appears to be taking on multiple distinct
responsibilities, which makes the overall design feel somewhat
awkward.

Perhaps this could be improved as part of your future inner-join removal patch.

--
Thanks,
Tender Wang

On Wed, Jun 17, 2026 at 11:00 AM Tender Wang <tndrwang@gmail.com> wrote:

In remove_rel_from_eclass(), we have
...
if (!IsA(em->em_expr, Var))
em->em_expr = (Expr *)
remove_rel_from_phvs((Node *) em->em_expr, relid, ojrelid);
...
"where s.id = 1" in the test case, the Const node seems to be able to skip, too.

Good point. Patch updated.

I have another question, although it's unrelated to this bug.
The logic in remove_rel_from_query() makes me a bit uneasy. It
currently has to handle both self-join and outer-join cases,
and it seems possible that inner-join-specific handling could end up
being added there in the future as well.
The function appears to be taking on multiple distinct
responsibilities, which makes the overall design feel somewhat
awkward.

Yeah. remove_rel_from_query() was originally written for left-join
removal only. The self-join elimination (SJE) commit grafted
self-join removal onto it, which made it do more than one thing within
one function. Commit 20efbdffe cleaned it up, clarifying the
separation between the left-join removal and self-join elimination
code paths within it. That cleanup was also meant to make it better
structured for adding new types of join removal, such as inner-join
removal, in the future.

That said, I agree that the current shape is not ideal. A better
structure might be to make remove_rel_from_query() handle only the
bookkeeping that is common to all removal types, and push the
type-specific work into the per-type callers. I think that is worth
pursuing as separate future work, kept apart from the current bug fix.

- Richard

Richard Guo <guofenglinux@gmail.com> 于2026年6月17日周三 13:51写道：

On Wed, Jun 17, 2026 at 11:00 AM Tender Wang <tndrwang@gmail.com> wrote:

In remove_rel_from_eclass(), we have
...
if (!IsA(em->em_expr, Var))
em->em_expr = (Expr *)
remove_rel_from_phvs((Node *) em->em_expr, relid, ojrelid);
...
"where s.id = 1" in the test case, the Const node seems to be able to skip, too.

Good point. Patch updated.

In remove_rel_from_phvs_mutator(), we have:

else if (IsA(node, Query))
{
Query *newnode;

...
}

I want to find what kind of SQL can enter the above else-if block.
So I added "assert(0)" to the else-if block. But no regression test crashed.
We test rel->baserestrictinfo here. Is it possible that the clause
includes a Query structure?
I know some xxx_mutator() functions in the planner have the same logic
processing pattern.
Is there a need to add a test case to cover the above code?

Yeah. remove_rel_from_query() was originally written for left-join
removal only. The self-join elimination (SJE) commit grafted
self-join removal onto it, which made it do more than one thing within
one function. Commit 20efbdffe cleaned it up, clarifying the
separation between the left-join removal and self-join elimination
code paths within it. That cleanup was also meant to make it better
structured for adding new types of join removal, such as inner-join
removal, in the future.

That said, I agree that the current shape is not ideal. A better
structure might be to make remove_rel_from_query() handle only the
bookkeeping that is common to all removal types, and push the
type-specific work into the per-type callers. I think that is worth
pursuing as separate future work, kept apart from the current bug fix.

Agree.

--
Thanks,
Tender Wang

On Thu, Jun 18, 2026 at 10:01 PM Tender Wang <tndrwang@gmail.com> wrote:

In remove_rel_from_phvs_mutator(), we have:

else if (IsA(node, Query))
{
Query *newnode;

...
}

I want to find what kind of SQL can enter the above else-if block.
So I added "assert(0)" to the else-if block. But no regression test crashed.
We test rel->baserestrictinfo here. Is it possible that the clause
includes a Query structure?

Good point. I considered this, and the answer is no. Any SubLink
that could contain a Query has already been expanded into a SubPlan by
the time we reach left-join removal, so we never encounter a Query
here.

I think what I had in mind when I wrote that branch was a SubLink
wrapped in a PHV with phlevelsup > 0, since SS_process_sublinks does
not recurse into the arguments of such an outer-level PHV. But that
case cannot arise here either: at left-join removal we only ever see
phlevelsup == 0 PHVs, because upper-level ones have already been
replaced with Params.

So we can drop the 'else if' branch. And we can remove sublevels_up
from remove_rel_from_phvs_context, and that makes it a one-field
struct, so we can remove the struct as well and just pass the Relids
directly.

Patch updated.

- Richard