MIME-Version: 1.0
References: 
 <CAJTYsWWzqpoRYxyA4ukjYpMPGEDgK2Za4t4wu9GdWOAtT8v-SQ@mail.gmail.com>
 <CAApHDvrJEM-qSm2rRJdowYtpss9xbmfRPRfOrEG0j01y1CkSNg@mail.gmail.com>
In-Reply-To: 
 <CAApHDvrJEM-qSm2rRJdowYtpss9xbmfRPRfOrEG0j01y1CkSNg@mail.gmail.com>
From: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Date: Mon, 13 Apr 2026 18:23:35 +0530
Message-ID: 
 <CAJTYsWXcd324VELk=9KdsfTsua9So3Yexqv7N3B23h9zAUD40g@mail.gmail.com>
Subject: Re: [PATCH] Fix NULL dereference in pg_get_database_ddl()
To: David Rowley <dgrowleyml@gmail.com>
Cc: pgsql-hackers@postgresql.org
Content-Type: multipart/alternative; boundary="000000000000393e74064f56f9c3"
Archived-At: 
 <https://www.postgresql.org/message-id/CAJTYsWXcd324VELk%3D9KdsfTsua9So3Yexqv7N3B23h9zAUD40g%40mail.gmail.com>
Precedence: bulk

--000000000000393e74064f56f9c3
Content-Type: text/plain; charset="UTF-8"

Hi,

On Mon, 13 Apr 2026 at 15:02, David Rowley <dgrowleyml@gmail.com> wrote:

> On Sat, 11 Apr 2026 at 01:58, Ayush Tiwari <ayushtiwari.slg01@gmail.com>
> wrote:
> > Deterministic reproduction:
> >
> > CREATE DATABASE regression_testdb;
> > SET allow_system_table_mods = on;
> > UPDATE pg_database
> >  SET dattablespace = 99999
> >  WHERE datname = 'regression_testdb';
> > RESET allow_system_table_mods;
> >
> > SELECT * FROM pg_get_database_ddl('regression_testdb');
> >
> > The attached patch fixes this by checking for NULL before calling
> > pg_strcasecmp().  In that case, pg_get_database_ddl() simply omits the
> > TABLESPACE clause.
>
> Can you explain why this method of self-inflicted catalogue corruption
> is any more important than any of the just-about-infinite other ways
> there are of causing issues by manually updating the catalogue tables?
>
> The typical response to this sort of thing can be seen in the thread
> in [1], in particular, the response in [2].
>
> David
>
> [1]
> https://www.postgresql.org/message-id/19383-e6b60ec2a4fce5b0@postgresql.org
> [2]
> https://www.postgresql.org/message-id/1538113.1768921841%40sss.pgh.pa.us


Thanks for the review.

I dug into this further and I don't think I can support the patch as a real
bug fix.

My original thought was that there might be a supported concurrent-DDL path
where
pg_get_database_ddl_internal() reads pg_database.dattablespace, and then a
concurrent change makes get_tablespace_name() return NULL before the second
lookup. I tried to reproduce that by widening the window and testing
ALTER DATABASE ... SET TABLESPACE together with DROP TABLESPACE, but I
could not
make it happen.

The reason seems to be that both catalog reads are governed by the same
statement-level CatalogSnapshot, so the function continues to see a
consistent
catalog view for the duration of the call. On the other side, DROP
TABLESPACE
also doesn't appear able to commit a state where pg_database still points
at a
vanished tablespace row through supported SQL, because the drop fails while
the
database directory is still present.

So at this point the remaining crash case seems to require a broken catalog
state,
which puts this in the same bucket as the precedent you cited rather than a
supported-path bug.

Given that, I don't think this patch is worth pursuing further in its
current
form. I'll drop it here unless I find a supported reproducer or a different,
stronger issue around the missing database/tablespace bookkeeping.

Thanks again for the push to investigate it more carefully.

Regards,
Ayush

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

<div dir=3D"ltr"><div dir=3D"ltr">Hi,</div><br><div class=3D"gmail_quote gm=
ail_quote_container"><div dir=3D"ltr" class=3D"gmail_attr">On Mon, 13 Apr 2=
026 at 15:02, David Rowley &lt;<a href=3D"mailto:dgrowleyml@gmail.com">dgro=
wleyml@gmail.com</a>&gt; wrote:<br></div><blockquote class=3D"gmail_quote" =
style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);pa=
dding-left:1ex">On Sat, 11 Apr 2026 at 01:58, Ayush Tiwari &lt;<a href=3D"m=
ailto:ayushtiwari.slg01@gmail.com" target=3D"_blank">ayushtiwari.slg01@gmai=
l.com</a>&gt; wrote:<br>
&gt; Deterministic reproduction:<br>
&gt;<br>
&gt; CREATE DATABASE regression_testdb;<br>
&gt; SET allow_system_table_mods =3D on;<br>
&gt; UPDATE pg_database<br>
&gt;=C2=A0 SET dattablespace =3D 99999<br>
&gt;=C2=A0 WHERE datname =3D &#39;regression_testdb&#39;;<br>
&gt; RESET allow_system_table_mods;<br>
&gt;<br>
&gt; SELECT * FROM pg_get_database_ddl(&#39;regression_testdb&#39;);<br>
&gt;<br>
&gt; The attached patch fixes this by checking for NULL before calling<br>
&gt; pg_strcasecmp().=C2=A0 In that case, pg_get_database_ddl() simply omit=
s the<br>
&gt; TABLESPACE clause.<br>
<br>
Can you explain why this method of self-inflicted catalogue corruption<br>
is any more important than any of the just-about-infinite other ways<br>
there are of causing issues by manually updating the catalogue tables?<br>
<br>
The typical response to this sort of thing can be seen in the thread<br>
in [1], in particular, the response in [2].<br>
<br>
David<br>
<br>
[1] <a href=3D"https://www.postgresql.org/message-id/19383-e6b60ec2a4fce5b0=
@postgresql.org" rel=3D"noreferrer" target=3D"_blank">https://www.postgresq=
l.org/message-id/19383-e6b60ec2a4fce5b0@postgresql.org</a><br>
[2] <a href=3D"https://www.postgresql.org/message-id/1538113.1768921841%40s=
ss.pgh.pa.us" rel=3D"noreferrer" target=3D"_blank">https://www.postgresql.o=
rg/message-id/1538113.1768921841%40sss.pgh.pa.us</a></blockquote><div><br><=
/div>Thanks for the review.<br><br>I dug into this further and I don&#39;t =
think I can support the patch as a real bug fix.<br><br>My original thought=
 was that there might be a supported concurrent-DDL path where<br>pg_get_da=
tabase_ddl_internal() reads pg_database.dattablespace, and then a<br>concur=
rent change makes get_tablespace_name() return NULL before the second<br>lo=
okup. I tried to reproduce that by widening the window and testing<br>ALTER=
 DATABASE ... SET TABLESPACE together with DROP TABLESPACE, but I could not=
<br>make it happen.<br><br>The reason seems to be that both catalog reads a=
re governed by the same<br>statement-level CatalogSnapshot, so the function=
 continues to see a consistent<br>catalog view for the duration of the call=
. On the other side, DROP TABLESPACE<br>also doesn&#39;t appear able to com=
mit a state where pg_database still points at a<br>vanished tablespace row =
through supported SQL, because the drop fails while the<br>database directo=
ry is still present.<br><br>So at this point the remaining crash case seems=
 to require a broken catalog state,</div><div class=3D"gmail_quote gmail_qu=
ote_container">which puts this in the same bucket as the precedent you cite=
d rather than a<br>supported-path bug.<br><br>Given that, I don&#39;t think=
 this patch is worth pursuing further in its current<br>form. I&#39;ll drop=
 it here unless I find a supported reproducer or a different,<br>stronger i=
ssue around the missing database/tablespace bookkeeping.<br><br><div>Thanks=
 again for the push to investigate it more carefully.=C2=A0<br><br>Regards,=
<br>Ayush</div></div></div>

--000000000000393e74064f56f9c3--