MIME-Version: 1.0
References: 
 <CA+3i_M8m1k2gFch+tU0JmAQh9FRV+pFrfTXDrJo+BqmwsTmOhg@mail.gmail.com>
 <CA+HiwqH+0rgMNQDog0AT9dVt0CGjXza_Li80njDhynWqzUwvZw@mail.gmail.com>
In-Reply-To: 
 <CA+HiwqH+0rgMNQDog0AT9dVt0CGjXza_Li80njDhynWqzUwvZw@mail.gmail.com>
From: Lakshmi N <lakshmin.jhs@gmail.com>
Date: Thu, 16 Apr 2026 09:46:45 -0700
Message-ID: 
 <CA+3i_M8Mq7fr6mUspSa2rLik9+oY4sOeD1qsB_-dvnUB4NrYbg@mail.gmail.com>
Subject: Re: Reject invalid databases in pg_get_database_ddl()
To: Amit Langote <amitlangote09@gmail.com>
Cc: "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>,
 andrew@dunslane.net
Content-Type: multipart/mixed; boundary="0000000000008f7274064f96942c"
Archived-At: 
 <https://www.postgresql.org/message-id/CA%2B3i_M8Mq7fr6mUspSa2rLik9%2BoY4sOeD1qsB_-dvnUB4NrYbg%40mail.gmail.com>
Precedence: bulk

--0000000000008f7274064f96942c
Content-Type: multipart/alternative; boundary="0000000000008f7273064f96942a"

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

Hi Amit,

On Thu, Apr 16, 2026 at 2:29=E2=80=AFAM Amit Langote <amitlangote09@gmail.c=
om>
wrote:

> Hi,
>
> On Thu, Apr 16, 2026 at 5:20=E2=80=AFPM Lakshmi N <lakshmin.jhs@gmail.com=
> wrote:
> > pg_get_database_ddl() is not checking for databases in an invalid state
> > before producing ddl statements. This caused the function to emit
> > CONNECTION_LIMIT =3D -2, which is invalid SQL that Postgres rejects.
> > A database row can be in this inconsistent state longer, for example
> > server crashed during a drop database.
> >
> > Attached patch to fix this issue by doing a database_is_invalid_form()
> > check early in pg_get_database_ddl_internal().
>
> Thanks for the report.
>
> Hmm, I see that the function will happily emit datconnlimit =3D -2 and
> your patch catches that at the top instead of down below near this
> code:
>
> /* CONNECTION LIMIT */
> if (dbform->datconnlimit !=3D -1)
> {
>     resetStringInfo(&buf);
>     appendStringInfo(&buf, "ALTER DATABASE %s CONNECTION LIMIT =3D %d;",
>                      quote_identifier(dbname), dbform->datconnlimit);
>     statements =3D lappend(statements, pstrdup(buf.data));
> }
>
> which, I guess, makes sense.
>
> The comment is correct but could be more explicit:
>
>     /*
>      * Reject invalid databases: datconnlimit =3D -2 would be emitted as
>      * CONNECTION LIMIT =3D -2, which fails on replay.
>      */
>
Thank you for reviewing! Please find the attached v2 addressing this.

Regards,
Lakshmi

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

<div dir=3D"ltr"><div dir=3D"ltr">Hi Amit,</div><br><div class=3D"gmail_quo=
te gmail_quote_container"><div dir=3D"ltr" class=3D"gmail_attr">On Thu, Apr=
 16, 2026 at 2:29=E2=80=AFAM Amit Langote &lt;<a href=3D"mailto:amitlangote=
09@gmail.com">amitlangote09@gmail.com</a>&gt; wrote:<br></div><blockquote c=
lass=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px soli=
d rgb(204,204,204);padding-left:1ex">Hi,<br>
<br>
On Thu, Apr 16, 2026 at 5:20=E2=80=AFPM Lakshmi N &lt;<a href=3D"mailto:lak=
shmin.jhs@gmail.com" target=3D"_blank">lakshmin.jhs@gmail.com</a>&gt; wrote=
:<br>
&gt; pg_get_database_ddl() is not checking for databases in an invalid stat=
e<br>
&gt; before producing ddl statements. This caused the function to emit<br>
&gt; CONNECTION_LIMIT =3D -2, which is invalid SQL that Postgres rejects.<b=
r>
&gt; A database row can be in this inconsistent state longer, for example<b=
r>
&gt; server crashed during a drop database.<br>
&gt;<br>
&gt; Attached patch to fix this issue by doing a database_is_invalid_form()=
<br>
&gt; check early in pg_get_database_ddl_internal().<br>
<br>
Thanks for the report.<br>
<br>
Hmm, I see that the function will happily emit datconnlimit =3D -2 and<br>
your patch catches that at the top instead of down below near this<br>
code:<br>
<br>
/* CONNECTION LIMIT */<br>
if (dbform-&gt;datconnlimit !=3D -1)<br>
{<br>
=C2=A0 =C2=A0 resetStringInfo(&amp;buf);<br>
=C2=A0 =C2=A0 appendStringInfo(&amp;buf, &quot;ALTER DATABASE %s CONNECTION=
 LIMIT =3D %d;&quot;,<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0quote_identifier(dbname), dbform-&gt;datconnlimit);<br>
=C2=A0 =C2=A0 statements =3D lappend(statements, pstrdup(buf.data));<br>
}<br>
<br>
which, I guess, makes sense.<br>
<br>
The comment is correct but could be more explicit:<br>
<br>
=C2=A0 =C2=A0 /*<br>
=C2=A0 =C2=A0 =C2=A0* Reject invalid databases: datconnlimit =3D -2 would b=
e emitted as<br>
=C2=A0 =C2=A0 =C2=A0* CONNECTION LIMIT =3D -2, which fails on replay.<br>
=C2=A0 =C2=A0 =C2=A0*/<br></blockquote><div>Thank you for reviewing! Please=
 find the attached v2 addressing this.</div><div><br></div><div>Regards,</d=
iv><div>Lakshmi=C2=A0</div><div>=C2=A0</div></div></div>

--0000000000008f7273064f96942a--

--0000000000008f7274064f96942c
Content-Type: application/octet-stream;
	name="v2-0001-Reject-pg_get_database_ddl-for-invalid-databases.patch"
Content-Disposition: attachment;
	filename="v2-0001-Reject-pg_get_database_ddl-for-invalid-databases.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_mo1possj0>
X-Attachment-Id: f_mo1possj0

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL3V0aWxzL2FkdC9kZGx1dGlscy5jIGIvc3JjL2JhY2tl
bmQvdXRpbHMvYWR0L2RkbHV0aWxzLmMKaW5kZXggYzRmOWY4NmM0M2UuLjUzM2ZjOGM5ZjljIDEw
MDY0NAotLS0gYS9zcmMvYmFja2VuZC91dGlscy9hZHQvZGRsdXRpbHMuYworKysgYi9zcmMvYmFj
a2VuZC91dGlscy9hZHQvZGRsdXRpbHMuYwpAQCAtODg3LDYgKzg4NywxNyBAQCBwZ19nZXRfZGF0
YWJhc2VfZGRsX2ludGVybmFsKE9pZCBkYmlkLCBib29sIHByZXR0eSwKIAlkYmZvcm0gPSAoRm9y
bV9wZ19kYXRhYmFzZSkgR0VUU1RSVUNUKHR1cGxlKTsKIAlkYm5hbWUgPSBwc3RyZHVwKE5hbWVT
dHIoZGJmb3JtLT5kYXRuYW1lKSk7CiAKKwkvKgorCSAqIFJlamVjdCBpbnZhbGlkIGRhdGFiYXNl
czogZGF0Y29ubmxpbWl0ID0gLTIgd291bGQgYmUgZW1pdHRlZCBhcworCSAqIENPTk5FQ1RJT04g
TElNSVQgPSAtMiwgd2hpY2ggZmFpbHMgb24gcmVwbGF5LgorCSAqLworCWlmIChkYXRhYmFzZV9p
c19pbnZhbGlkX2Zvcm0oZGJmb3JtKSkKKwkJZXJlcG9ydChFUlJPUiwKKwkJCQkoZXJyY29kZShF
UlJDT0RFX09CSkVDVF9OT1RfSU5fUFJFUkVRVUlTSVRFX1NUQVRFKSwKKwkJCQkgZXJybXNnKCJj
YW5ub3QgZ2VuZXJhdGUgRERMIGZvciBpbnZhbGlkIGRhdGFiYXNlIFwiJXNcIiIsCisJCQkJCQlk
Ym5hbWUpLAorCQkJCSBlcnJoaW50KCJVc2UgRFJPUCBEQVRBQkFTRSB0byBkcm9wIGludmFsaWQg
ZGF0YWJhc2VzLiIpKSk7CisKIAkvKgogCSAqIFdlIGRvbid0IHN1cHBvcnQgZ2VuZXJhdGluZyBE
REwgZm9yIHN5c3RlbSBkYXRhYmFzZXMuICBUaGUgcHJpbWFyeQogCSAqIHJlYXNvbiBmb3IgdGhp
cyBpcyB0aGF0IHVzZXJzIHNob3VsZG4ndCBiZSByZWNyZWF0aW5nIHRoZW0uCg==
--0000000000008f7274064f96942c--