MIME-Version: 1.0
References: <CAMjNa7eFzTQ5=oZMQiB2bMkez5KP4A77JC7SRjeVEkOrh7cUHw@mail.gmail.com>
 <CACLU5mST1LhC3ibaKGNch_=06S2cmbjR4PnoUSupKs+rtgdeyw@mail.gmail.com> <CAMjNa7fJUwcOxf+qV8g+tCQ-3E-YAiKgE_Qs6u-xjdxe12T0SQ@mail.gmail.com>
In-Reply-To: <CAMjNa7fJUwcOxf+qV8g+tCQ-3E-YAiKgE_Qs6u-xjdxe12T0SQ@mail.gmail.com>
From: Adam Brusselback <adambrusselback@gmail.com>
Date: Sat, 3 Jan 2026 21:55:46 -0500
Message-ID: <CAMjNa7egcgUMf2tdQ1qeTYj1J1bBvyth3thoZPioujusFsBd4Q@mail.gmail.com>
Subject: Re: [Patch] Add WHERE clause support to REFRESH MATERIALIZED VIEW
To: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/alternative; boundary="0000000000002e31220647871593"
Archived-At: <https://www.postgresql.org/message-id/CAMjNa7egcgUMf2tdQ1qeTYj1J1bBvyth3thoZPioujusFsBd4Q%40mail.gmail.com>
Precedence: bulk

--0000000000002e31220647871593
Content-Type: text/plain; charset="UTF-8"

Hi all,

I've been running some more concurrency tests against this patch
(specifically looking for race conditions), and I found a flaw in the
implementation for the  REFRESH ... WHERE ... mode (without CONCURRENTLY).

I believe the issue is that the DELETE -> INSERT strategy leaves a
consistency gap. Since we relied on ROW EXCLUSIVE locks to allow concurrent
reads, the moment we delete the rows, we lose the physical lock on them. If
a concurrent transaction inserts a colliding row during that gap, the
materialized view ends up inconsistent with the base query (or hits a
constraint violation).

I initially was using SELECT ... FOR UPDATE to lock the rows before
modification, but that lock is (now that I know) obviously lost when the
row is deleted.

My plan is to replace that row-locking strategy with transaction-level
advisory locks inside the refresh logic:

Before the DELETE, run a SELECT pg_advisory_xact_lock(mv_oid,
hashtext(ROW(unique_keys)::text)) for the rows matching the WHERE clause.

This effectively locks the "logical" ID of the row, preventing concurrent
refreshes on the same ID even while the physical tuple is temporarily gone.
Hash collisions should not have any correctness issues that I can think of.

However, before I sink time into implementing that fix:

Is there general interest in having REFRESH MATERIALIZED VIEW ... WHERE ...
in core?
If the community feels this feature is a footgun or conceptually wrong for
Postgres, I'd rather know now before spending more time on this.

If the feature concept is sound, does the advisory lock approach seem like
the right way to handle the concurrency safety here?

Thanks,
Adam Brusselback

--0000000000002e31220647871593
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr">Hi all,<br><br>I&#39;ve been running some more concurrency=
 tests against this patch (specifically looking for race conditions), and I=
 found a flaw in the implementation for the=C2=A0 REFRESH ... WHERE ... mod=
e (without CONCURRENTLY).<br><br>I believe the issue is that the DELETE -&g=
t; INSERT strategy leaves a consistency gap. Since we relied on ROW EXCLUSI=
VE locks to allow concurrent reads, the moment we delete the rows, we lose =
the physical lock on them. If a concurrent transaction inserts a colliding =
row during that gap, the materialized view ends up inconsistent with the ba=
se query (or hits a constraint violation).<br><br>I initially was using SEL=
ECT ... FOR UPDATE to lock the rows before modification, but that lock is (=
now that I know) obviously lost when the row is deleted.<br><br>My plan is =
to replace that row-locking strategy with transaction-level advisory locks =
inside the refresh logic:<br><br>Before the DELETE, run a SELECT pg_advisor=
y_xact_lock(mv_oid, hashtext(ROW(unique_keys)::text)) for the rows matching=
 the WHERE clause.<br><br>This effectively locks the &quot;logical&quot; ID=
 of the row, preventing concurrent refreshes on the same ID even while the =
physical tuple is temporarily gone. Hash collisions should not have any cor=
rectness issues that I can think of.<br><br>However, before I sink time int=
o implementing that fix:<br><br>Is there general interest in having REFRESH=
 MATERIALIZED VIEW ... WHERE ... in core?<br>If the community feels this fe=
ature is a footgun or conceptually wrong for Postgres, I&#39;d rather know =
now before spending more time on this.<br><br>If the feature concept is sou=
nd, does the advisory lock approach seem like the right way to handle the c=
oncurrency safety here?<br><br>Thanks,<br>Adam Brusselback</div>

--0000000000002e31220647871593--