MIME-Version: 1.0
References: <33766.1780471821@localhost> <aiCUGyksPoXXolhk@alvherre.pgsql>
In-Reply-To: <aiCUGyksPoXXolhk@alvherre.pgsql>
From: Srinath Reddy Sadipiralla <srinath2133@gmail.com>
Date: Fri, 5 Jun 2026 11:39:03 +0530
Message-ID: 
 <CAFC+b6q6v+3jwcrZRO-dqj4KXy6uGaxi7qwoOC0cOOD5DeJn3Q@mail.gmail.com>
Subject: Re: BUG #19500: pgrepack logical decoding plugin can crash assert
 builds via SQL decoding API
To: Alvaro Herrera <alvherre@kurilemu.de>
Cc: Antonin Houska <ah@cybertec.at>, n.kalinin@postgrespro.ru,
	pgsql-bugs@lists.postgresql.org
Content-Type: multipart/alternative; boundary="0000000000000ad62806537b80f2"
Archived-At: 
 <https://www.postgresql.org/message-id/CAFC%2Bb6q6v%2B3jwcrZRO-dqj4KXy6uGaxi7qwoOC0cOOD5DeJn3Q%40mail.gmail.com>
Precedence: bulk

--0000000000000ad62806537b80f2
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Jun 4, 2026 at 2:50=E2=80=AFAM Alvaro Herrera <alvherre@kurilemu.de=
> wrote:

> On 2026-Jun-03, Antonin Houska wrote:
>
> > Srinath Reddy Sadipiralla <srinath2133@gmail.com> wrote:
> >
> > > Could we reject the pgrepack plugin at slot creation instead, in
> > > pg_create_logical_replication_slot() and the CREATE_REPLICATION_SLOT
> > > command, so misuse gets a clear "reserved for REPACK (CONCURRENTLY)"
> > > error up front, before any decoding? REPACK creates its slot directly
> via
> > > ReplicationSlotCreate(), so it's unaffected, and the begin-callback
> check
> > > with magic guard can stay as the internal safety net.
> > > Happy to be told this isn't worth special-casing :)
> >
> > Another possible approach: restrict the use of the plugin to the REPACK
> > decoding worker.
>
> I don't like either of these approaches, because they are forcing the
> generic facility (either slot creation or logical decoding setup) to
> know something about one specific user of the facility.  That is to say,
> the restriction is being added on the wrong side of the abstraction.
> I know my implementation the drawback you (Srinath) mentioned, because
> the abstraction doesn't provide us with a great way to inject an error
> report at the exact spot we need it; but I think it's at the correct
> side of the abstraction.


> (I'm not really sure that there _is_ a great way to throw an error
> report at the right time.  That would require every single output plugin
> author to add a function we can call; and every single one of them,
> except REPACK, would do nothing.  This seems quite pointless.)
>
> I frankly don't have a problem with letting a transaction spill a few
> GBs to disk only to then report an error that pgrepack is being misused.
> It's just not something that anyone would do for fun.
>

makes sense, we can go with your approach, thanks for
the clarification.


--=20
Thanks :)
Srinath Reddy Sadipiralla
EDB: https://www.enterprisedb.com/

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

<div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote g=
mail_quote_container"><div dir=3D"ltr" class=3D"gmail_attr">On Thu, Jun 4, =
2026 at 2:50=E2=80=AFAM Alvaro Herrera &lt;<a href=3D"mailto:alvherre@kuril=
emu.de">alvherre@kurilemu.de</a>&gt; wrote:<br></div><blockquote class=3D"g=
mail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204=
,204,204);padding-left:1ex">On 2026-Jun-03, Antonin Houska wrote:<br>
<br>
&gt; Srinath Reddy Sadipiralla &lt;<a href=3D"mailto:srinath2133@gmail.com"=
 target=3D"_blank">srinath2133@gmail.com</a>&gt; wrote:<br>
&gt; <br>
&gt; &gt; Could we reject the pgrepack plugin at slot creation instead, in<=
br>
&gt; &gt; pg_create_logical_replication_slot() and the CREATE_REPLICATION_S=
LOT<br>
&gt; &gt; command, so misuse gets a clear &quot;reserved for REPACK (CONCUR=
RENTLY)&quot;<br>
&gt; &gt; error up front, before any decoding? REPACK creates its slot dire=
ctly via<br>
&gt; &gt; ReplicationSlotCreate(), so it&#39;s unaffected, and the begin-ca=
llback check<br>
&gt; &gt; with magic guard can stay as the internal safety net.<br>
&gt; &gt; Happy to be told this isn&#39;t worth special-casing :)<br>
&gt; <br>
&gt; Another possible approach: restrict the use of the plugin to the REPAC=
K<br>
&gt; decoding worker.<br>
<br>
I don&#39;t like either of these approaches, because they are forcing the<b=
r>
generic facility (either slot creation or logical decoding setup) to<br>
know something about one specific user of the facility.=C2=A0 That is to sa=
y,<br>
the restriction is being added on the wrong side of the abstraction.<br>
I know my implementation the drawback you (Srinath) mentioned, because<br>
the abstraction doesn&#39;t provide us with a great way to inject an error<=
br>
report at the exact spot we need it; but I think it&#39;s at the correct<br=
>
side of the abstraction.</blockquote><blockquote class=3D"gmail_quote" styl=
e=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);paddin=
g-left:1ex">
<br>
(I&#39;m not really sure that there _is_ a great way to throw an error<br>
report at the right time.=C2=A0 That would require every single output plug=
in<br>
author to add a function we can call; and every single one of them,<br>
except REPACK, would do nothing.=C2=A0 This seems quite pointless.)<br>
<br>
I frankly don&#39;t have a problem with letting a transaction spill a few<b=
r>
GBs to disk only to then report an error that pgrepack is being misused.<br=
>
It&#39;s just not something that anyone would do for fun.<br></blockquote><=
div><br>makes sense, we can go with your approach, thanks for<br>the clarif=
ication.</div></div><div><br clear=3D"all"></div><div><br></div><span class=
=3D"gmail_signature_prefix">-- </span><br><div dir=3D"ltr" class=3D"gmail_s=
ignature"><div dir=3D"ltr"><span style=3D"color:rgb(34,34,34)">Thanks :)</s=
pan><br style=3D"color:rgb(34,34,34)"><span style=3D"color:rgb(34,34,34)">S=
rinath Reddy Sadipiralla</span><br style=3D"color:rgb(34,34,34)"><span styl=
e=3D"color:rgb(34,34,34)">EDB:=C2=A0</span><a href=3D"https://www.enterpris=
edb.com/" style=3D"color:rgb(17,85,204)" target=3D"_blank">https://www.ente=
rprisedb.com/</a></div></div></div>

--0000000000000ad62806537b80f2--