Reject invalid databases in pg_get_database_ddl()

Hi,

On Thu, Apr 16, 2026 at 5:20 PM 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 = -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 = -2 and
your patch catches that at the top instead of down below near this
code:

/* CONNECTION LIMIT */
if (dbform->datconnlimit != -1)
{
resetStringInfo(&buf);
appendStringInfo(&buf, "ALTER DATABASE %s CONNECTION LIMIT = %d;",
quote_identifier(dbname), dbform->datconnlimit);
statements = lappend(statements, pstrdup(buf.data));
}

which, I guess, makes sense.

The comment is correct but could be more explicit:

/*
* Reject invalid databases: datconnlimit = -2 would be emitted as
* CONNECTION LIMIT = -2, which fails on replay.
*/

--
Thanks, Amit Langote

Hi Amit,

On Thu, Apr 16, 2026 at 2:29 AM Amit Langote <amitlangote09@gmail.com>
wrote:

Hi,

On Thu, Apr 16, 2026 at 5:20 PM 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 = -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 = -2 and
your patch catches that at the top instead of down below near this
code:

/* CONNECTION LIMIT */
if (dbform->datconnlimit != -1)
{
resetStringInfo(&buf);
appendStringInfo(&buf, "ALTER DATABASE %s CONNECTION LIMIT = %d;",
quote_identifier(dbname), dbform->datconnlimit);
statements = lappend(statements, pstrdup(buf.data));
}

which, I guess, makes sense.

The comment is correct but could be more explicit:

/*
* Reject invalid databases: datconnlimit = -2 would be emitted as
* CONNECTION LIMIT = -2, which fails on replay.
*/

Thank you for reviewing! Please find the attached v2 addressing this.

Regards,
Lakshmi