MIME-Version: 1.0
References: 
 <CA+3i_M8m1k2gFch+tU0JmAQh9FRV+pFrfTXDrJo+BqmwsTmOhg@mail.gmail.com>
 <CA+HiwqH+0rgMNQDog0AT9dVt0CGjXza_Li80njDhynWqzUwvZw@mail.gmail.com>
 <CA+3i_M8Mq7fr6mUspSa2rLik9+oY4sOeD1qsB_-dvnUB4NrYbg@mail.gmail.com>
In-Reply-To: 
 <CA+3i_M8Mq7fr6mUspSa2rLik9+oY4sOeD1qsB_-dvnUB4NrYbg@mail.gmail.com>
From: Amit Langote <amitlangote09@gmail.com>
Date: Fri, 17 Apr 2026 08:46:07 +0900
Message-ID: 
 <CA+HiwqFW7uY3Jknf8VzjWoXa1bf0-U6-+JgM+o1ggJ+onyOHHA@mail.gmail.com>
Subject: Re: Reject invalid databases in pg_get_database_ddl()
To: Lakshmi N <lakshmin.jhs@gmail.com>
Cc: "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>,
 andrew@dunslane.net
Content-Type: multipart/mixed; boundary="000000000000b342f6064f9c7072"
Archived-At: 
 <https://www.postgresql.org/message-id/CA%2BHiwqFW7uY3Jknf8VzjWoXa1bf0-U6-%2BJgM%2Bo1ggJ%2BonyOHHA%40mail.gmail.com>
Precedence: bulk

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

On Fri, Apr 17, 2026 at 1:46=E2=80=AFAM Lakshmi N <lakshmin.jhs@gmail.com> =
wrote:
> On Thu, Apr 16, 2026 at 2:29=E2=80=AFAM Amit Langote <amitlangote09@gmail=
.com> wrote:
>>
>> Hi,
>>
>> On Thu, Apr 16, 2026 at 5:20=E2=80=AFPM Lakshmi N <lakshmin.jhs@gmail.co=
m> wrote:
>> > pg_get_database_ddl() is not checking for databases in an invalid stat=
e
>> > 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.

Thanks.  Will push the attached shortly.

--=20
Thanks, Amit Langote

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

RnJvbSBjYTk4MDg3NzM0ZGYwY2RkZWZkOTYxMGI3YzIzYmU4ZDFiNzU4OTY5IE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBBbWl0IExhbmdvdGUgPGFtaXRsYW5AcG9zdGdyZXNxbC5vcmc+
CkRhdGU6IEZyaSwgMTcgQXByIDIwMjYgMDg6NDI6MTkgKzA5MDAKU3ViamVjdDogW1BBVENIIHYy
XSBSZWplY3QgaW52YWxpZCBkYXRhYmFzZXMgaW4gcGdfZ2V0X2RhdGFiYXNlX2RkbCgpCgpBbiBp
bnZhbGlkIGRhdGFiYXNlIGhhcyBkYXRjb25ubGltaXQgc2V0IHRvIC0yLiAgcGdfZ2V0X2RhdGFi
YXNlX2RkbCgpCmVtaXRzIHRoaXMgdmVyYmF0aW0gYXMgQ09OTkVDVElPTiBMSU1JVCA9IC0yLCB3
aGljaCBBTFRFUiBEQVRBQkFTRQpyZWplY3RzLiAgRXJyb3Igb3V0IGVhcmx5IGluc3RlYWQuCgpS
ZXBvcnRlZC1ieTogTGFrc2htaSBOIDxsYWtzaG1pbi5qaHNAZ21haWwuY29tPgpBdXRob3I6IExh
a3NobWkgTiA8bGFrc2htaW4uamhzQGdtYWlsLmNvbT4KUmV2aWV3ZWQtYnk6IEFtaXQgTGFuZ290
ZSA8YW1pdGxhbmdvdGUwOUBnbWFpbC5jb20+CkRpc2N1c3Npb246IGh0dHBzOi8vcG9zdGdyLmVz
L20vQ0ErM2lfTThtMWsyZ0ZjaCt0VTBKbUFRaDlGUlYrcEZyZlRYRHJKbytCcW13c1RtT2hnQG1h
aWwuZ21haWwuY29tCi0tLQogc3JjL2JhY2tlbmQvdXRpbHMvYWR0L2RkbHV0aWxzLmMgfCAxMSAr
KysrKysrKysrKwogMSBmaWxlIGNoYW5nZWQsIDExIGluc2VydGlvbnMoKykKCmRpZmYgLS1naXQg
YS9zcmMvYmFja2VuZC91dGlscy9hZHQvZGRsdXRpbHMuYyBiL3NyYy9iYWNrZW5kL3V0aWxzL2Fk
dC9kZGx1dGlscy5jCmluZGV4IGM0ZjlmODZjNDNlLi4zOTIxZWQxZmE2YSAxMDA2NDQKLS0tIGEv
c3JjL2JhY2tlbmQvdXRpbHMvYWR0L2RkbHV0aWxzLmMKKysrIGIvc3JjL2JhY2tlbmQvdXRpbHMv
YWR0L2RkbHV0aWxzLmMKQEAgLTg4Nyw2ICs4ODcsMTcgQEAgcGdfZ2V0X2RhdGFiYXNlX2RkbF9p
bnRlcm5hbChPaWQgZGJpZCwgYm9vbCBwcmV0dHksCiAJZGJmb3JtID0gKEZvcm1fcGdfZGF0YWJh
c2UpIEdFVFNUUlVDVCh0dXBsZSk7CiAJZGJuYW1lID0gcHN0cmR1cChOYW1lU3RyKGRiZm9ybS0+
ZGF0bmFtZSkpOwogCisJLyoKKwkgKiBSZWplY3QgaW52YWxpZCBkYXRhYmFzZXM6IGRhdGNvbm5s
aW1pdCA9IC0yIHdvdWxkIGJlIGVtaXR0ZWQgYXMKKwkgKiBDT05ORUNUSU9OIExJTUlUID0gLTIs
IHdoaWNoIGNhbm5vdCBiZSBleGVjdXRlZC4KKwkgKi8KKwlpZiAoZGF0YWJhc2VfaXNfaW52YWxp
ZF9mb3JtKGRiZm9ybSkpCisJCWVyZXBvcnQoRVJST1IsCisJCQkJKGVycmNvZGUoRVJSQ09ERV9P
QkpFQ1RfTk9UX0lOX1BSRVJFUVVJU0lURV9TVEFURSksCisJCQkJIGVycm1zZygiY2Fubm90IGdl
bmVyYXRlIERETCBmb3IgaW52YWxpZCBkYXRhYmFzZSBcIiVzXCIiLAorCQkJCQkJZGJuYW1lKSwK
KwkJCQkgZXJyaGludCgiVXNlIERST1AgREFUQUJBU0UgdG8gZHJvcCBpbnZhbGlkIGRhdGFiYXNl
cy4iKSkpOworCiAJLyoKIAkgKiBXZSBkb24ndCBzdXBwb3J0IGdlbmVyYXRpbmcgRERMIGZvciBz
eXN0ZW0gZGF0YWJhc2VzLiAgVGhlIHByaW1hcnkKIAkgKiByZWFzb24gZm9yIHRoaXMgaXMgdGhh
dCB1c2VycyBzaG91bGRuJ3QgYmUgcmVjcmVhdGluZyB0aGVtLgotLSAKMi40Ny4zCgo=
--000000000000b342f6064f9c7072--