public inbox for [email protected]
help / color / mirror / Atom feedFrom: Ayush Tiwari <[email protected]>
To: David Rowley <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] Fix NULL dereference in pg_get_database_ddl()
Date: Mon, 13 Apr 2026 18:23:35 +0530
Message-ID: <CAJTYsWXcd324VELk=9KdsfTsua9So3Yexqv7N3B23h9zAUD40g@mail.gmail.com> (raw)
In-Reply-To: <CAApHDvrJEM-qSm2rRJdowYtpss9xbmfRPRfOrEG0j01y1CkSNg@mail.gmail.com>
References: <CAJTYsWWzqpoRYxyA4ukjYpMPGEDgK2Za4t4wu9GdWOAtT8v-SQ@mail.gmail.com>
<CAApHDvrJEM-qSm2rRJdowYtpss9xbmfRPRfOrEG0j01y1CkSNg@mail.gmail.com>
Hi,
On Mon, 13 Apr 2026 at 15:02, David Rowley <[email protected]> wrote:
> On Sat, 11 Apr 2026 at 01:58, Ayush Tiwari <[email protected]>
> 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/[email protected]
> [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
view thread (4+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: [PATCH] Fix NULL dereference in pg_get_database_ddl()
In-Reply-To: <CAJTYsWXcd324VELk=9KdsfTsua9So3Yexqv7N3B23h9zAUD40g@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox