Tom Lane <[email protected]> 于2026年10月25日周日 08:56写道：
>
> So I still don't see a reachable bug here, but there's dead code
> (the bit about handling an rmark without kmark), and the
> commentary omits even more than I thought before.
>
Thanks for the very detailed analysis. I tested the cases you
described, and you are right: my original example does not demonstrate
a reachable correctness bug.
Following your suggestion, the attached v2 patch removes the dead code
for transferring an rmark when there is no kmark, adds assertions for
that invariant, and expands the comments about why relations with
different row mark types are not currently merged.
> I experimented with writing logic that would allow merging locks of
> different strengths, but I'm not really convinced that it's worth the
> trouble.
I haven't included the other improvement you suggested—performing SJE
when lock strengths differ and retaining the stronger lock type, along
with an appropriate rule for choosing the waitPolicy.
I think it would be better to consider that separately from this cleanup patch.
During testing, I also noticed another possible missed optimization.
For example, adding a restriction such as:
postgres=# EXPLAIN (VERBOSE, COSTS OFF)
SELECT p.a, q.a
FROM t1 p
JOIN t1 q ON p.a = q.a
WHERE p.a = 18;
QUERY PLAN
-------------------------------
Nested Loop
Output: p.a, q.a
-> Seq Scan on public.t1 p
Output: p.a, p.b
Filter: (p.a = 18)
-> Seq Scan on public.t1 q
Output: q.a, q.b
Filter: (q.a = 18)
(8 rows)
to the self-join query prevents SJE, whereas removing that restriction
allows SJE to occur.
It appears that the equivalence class containing the constant absorbs
the equality between p.a and q.a, so
generate_join_implied_equalities() returns NIL and SJE gives up.
Do you think this case is worth improving, perhaps as a separate patch?
--
Thanks,
Tender Wang
From 46212f3111b0290ded005ab3c6e3bc5d914742c8 Mon Sep 17 00:00:00 2001
From: Tender Wang <[email protected]>
Date: Mon, 26 Oct 2026 10:59:30 +0800
Subject: [PATCH v2] Clarify row mark handling in self-join elimination
When row marks are needed, preprocess_rowmarks() creates one for every
non-target base relation. Since target relations are not candidates for
self-join elimination, a pair of candidate relations must either both
have row marks or both lack them.
Therefore, the code that transfers the removed relation's row mark when
the kept relation has none is unreachable. Remove that code, add
assertions for the invariant, and expand the comments explaining why
relations with different row mark types are not merged.
---
src/backend/optimizer/plan/analyzejoins.c | 40 +++++++++--------------
1 file changed, 16 insertions(+), 24 deletions(-)
diff --git a/src/backend/optimizer/plan/analyzejoins.c
b/src/backend/optimizer/plan/analyzejoins.c
index 7aad91fd34e..dcfe5097586 100644
--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -1287,28 +1287,13 @@ remove_self_join_rel(PlannerInfo *root,
Assert(found_relid);
/*
- * If the removed relation has a row mark, transfer it to the remaining
- * one.
- *
- * If both rels have row marks, just keep the one corresponding to the
- * remaining relation because we verified earlier that they have the
same
- * strength.
+ * If the relations have row marks, discard the removed relation's mark.
+ * The caller has verified that both marks exist and have the same type.
*/
if (rmark)
{
- if (kmark)
- {
- Assert(kmark->markType == rmark->markType);
-
- root->rowMarks = list_delete_ptr(root->rowMarks, rmark);
- }
- else
- {
- /* Shouldn't have inheritance children yet. */
- Assert(rmark->rti == rmark->prti);
-
- rmark->rti = rmark->prti = toKeep->relid;
- }
+ Assert(rmark->markType == kmark->markType);
+ root->rowMarks = list_delete_ptr(root->rowMarks, rmark);
}
}
@@ -1827,10 +1812,16 @@ remove_self_joins_one_group(PlannerInfo *root, Relids
relids)
continue;
/*
- * Check Row Marks equivalence. We can't remove the
join if the
- * relations have row marks of different strength
(e.g., one is
- * locked FOR UPDATE, and another just has
ROW_MARK_REFERENCE for
- * EvalPlanQual rechecking).
+ * If row marks are needed, preprocess_rowmarks()
creates one for
+ * every non-target base relation. Target relations
aren't SJE
+ * candidates, so these relations must either both have
row marks or
+ * both lack them.
+ *
+ * Don't merge relations with different row mark types.
In
+ * particular, a locking row mark can fetch an updated
version of a
+ * tuple, while a non-locking row mark preserves the
version found by
+ * the original scan. We also conservatively reject
different locking
+ * row mark types rather than try to combine their lock
strengths.
*/
foreach(lc, root->rowMarks)
{
@@ -1850,7 +1841,8 @@ remove_self_joins_one_group(PlannerInfo *root, Relids
relids)
if (kmark && rmark)
break;
}
- if (kmark && rmark && kmark->markType !=
rmark->markType)
+ Assert((kmark == NULL) == (rmark == NULL));
+ if (kmark && kmark->markType != rmark->markType)
continue;
/*
--
2.43.0
