Self-join elimination discards differing row lock wait policies
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t254368psql -h localhost -U postgresBuilt from patchset v1 (message #1), October 24, 2026 at 08:16 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t254368_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t254368_1 && git checkout t254368_1Patchset v1 (message #1) is on t254368_1
Hi,
Self-join elimination discards one of the row lock wait policies when
two aliases of the same relation use different policies.
How to reproduce:
CREATE TABLE t1 (a int PRIMARY KEY, b int);
INSERT INTO t1 SELECT i, (i * 7) % 13 FROM generate_series(1, 40) i;
ANALYZE t1;
In session 1, lock the row where a = 5:
postgres=# begin;
BEGIN
postgres=*# SELECT a FROM t1 WHERE a=5 FOR UPDATE;
a
---
5
(1 row)
In session 2:
postgres=# show enable_self_join_elimination ;
enable_self_join_elimination
------------------------------
on
(1 row)
postgres=# SELECT p.a, q.a FROM t1 p JOIN t1 q ON p.a = q.a
WHERE p.b = 9 AND q.b = 9
FOR UPDATE OF p NOWAIT FOR UPDATE OF q SKIP LOCKED;
a | a
----+----
18 | 18
31 | 31
(2 rows)
postgres=# set enable_self_join_elimination = off;
SET
postgres=# SELECT p.a, q.a FROM t1 p JOIN t1 q ON p.a = q.a
WHERE p.b = 9 AND q.b = 9
FOR UPDATE OF p NOWAIT FOR UPDATE OF q SKIP LOCKED;
ERROR: could not obtain lock on row in relation "t1"
The two aliases have PlanRowMarks with the same markType and locking
strength, but different waitPolicy values. The row mark equivalence
check in remove_self_joins_one_group() currently compares only markType.
Self-join elimination is therefore allowed, and remove_self_join_rel()
discards the removed relation's PlanRowMark.
The query is then executed using only the wait policy of the retained relation.
In this example, the NOWAIT behavior is lost, and the locked row is
handled using SKIP LOCKED.
Reversing the wait-policy order exposes the opposite behavior change.
A blocking policy can be lost in the same way.
The attached patch requires the row mark type, locking strength, and
wait policy to match before eliminating the self-join.
Thoughts?
--
Thanks,
Tender Wang
Tender Wang <tndrwang@gmail.com> writes:
Self-join elimination discards one of the row lock wait policies when
two aliases of the same relation use different policies.
Good catch.
The attached patch requires the row mark type, locking strength, and
wait policy to match before eliminating the self-join.
Thoughts?
In some cases it might be possible to merge two "sufficiently similar"
rowmark requirements, eg keep only the stronger lock type. I'm not
sure that that wouldn't produce visible semantics changes, though.
On the other hand, the code is already willing to merge an RTE with
a RowMark with an RTE that has none, and I'm not convinced that that
has no visible semantics effects either --- for example, if the one
RowMark says "SKIP LOCKED", doesn't it represent a different set of
rows than an RTE with no RowMark? Maybe we should tighten this even
more and reject that case?
regards, tom lane
I wrote:
Tender Wang <tndrwang@gmail.com> writes:
The attached patch requires the row mark type, locking strength, and
wait policy to match before eliminating the self-join.
Thoughts?
In some cases it might be possible to merge two "sufficiently similar"
rowmark requirements, eg keep only the stronger lock type. I'm not
sure that that wouldn't produce visible semantics changes, though.
On the other hand, the code is already willing to merge an RTE with
a RowMark with an RTE that has none, and I'm not convinced that that
has no visible semantics effects either --- for example, if the one
RowMark says "SKIP LOCKED", doesn't it represent a different set of
rows than an RTE with no RowMark? Maybe we should tighten this even
more and reject that case?
On closer consideration, there's no bug here.
1. We only support SJE on regular tables, not foreign tables.
Therefore, the requested lock strength is fully encoded in the
RowMarkType (see select_rowmark_type()), so checking that guarantees
that the strength field matches.
2. It doesn't matter whether the waitPolicy is the same, because
the behavior after SJE is isomorphic to a legal implementation
of the query without SJE. The kept waitPolicy determines what
actually happens for a given row, and it's mostly like what would
happen in a join query where the kept RTE scans a given row first:
LockWaitBlock: wait to acquire the lock. If we do that first,
and then the other RTE scans the row, it will be satisfied.
LockWaitSkip: skip the row. Even if the other RTE would have
acquired the lock, the row would not appear in the query result
since the equijoin would drop it. You could complain that
the other RTE had LockWaitError and would have thrown an error,
but it's legitimate for us to act as though we're doing this via
a nestloop and hence never scanned the other RTE for this row.
By the same argument, it's okay that we never acquired the lock,
even if the other RTE had LockWaitBlock.
LockWaitError: error if we don't get the lock. Does not matter
what the other RTE would have done when it got to the row.
3. By the same argument, my complaint above about two RTEs with
and without rowmarks is faulty. As long as we keep the rowmark,
dropping the RTE without rowmark is not problematic, because
each of the cases I just listed works the same if the second
RTE applies no row lock at all.
So I don't see anything that looks like a bug, although the
comments about this in analyzejoins.c are surely inadequate.
Having said that, there's still room for improvement here.
If we don't perform SJE because the lock strengths are different,
then the query's behavior depends on which order the planner
chooses to join the tables in. Which is not great, especially
since the user can't really control that, and there's a real risk
of deadlock failures due to attempted lock upgrade if we take the
weaker lock first. It would be better to perform SJE and make
sure we keep the stronger lock type, which generalizes what we're
doing now in the only-one-side-has-a-rowmark case. I think we
could also work out a rule for which waitPolicy to choose when
we do that, ensuring more consistent behavior than you get now.
regards, tom lane
I wrote:
3. By the same argument, my complaint above about two RTEs with
and without rowmarks is faulty. As long as we keep the rowmark,
dropping the RTE without rowmark is not problematic, because
each of the cases I just listed works the same if the second
RTE applies no row lock at all.
I realized that this argument is still faulty, because an RTE with a
locking-type rowmark potentially represents a different set of row
values than an RTE that has no mark or a ROW_MARK_REFERENCE mark
(i.e., fully-updated versus current-as-of-the-query-snapshot).
So we could potentially merge RTEs that have locking rowmarks of
different strengths, and we could potentially merge non-locking RTEs,
but we can't merge across those groups.
The existing test for same-markType keeps us from merging a
ROW_MARK_REFERENCE mark with a locking mark, but it's stronger
than it has to be. And there is a bug, which is that the code is
willing to merge an RTE with locking mark with an RTE that has no
mark. But it turns out that that bug is only latent, because
preprocess_rowmarks has previously seen to it that all or none of the
query's rels have PlanRowMarks. (Except for an UPDATE/DELETE target
rel, but analyzejoins.c rejects SJE with the target rel anyway.)
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.
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.
regards, tom lane