Re: Reject invalid databases in pg_get_database_ddl()

Hi, Lakshmi

On Thu, 16 Apr 2026 at 09:46, Lakshmi N <lakshmin(dot)jhs(at)gmail(dot)com> wrote:
> Hi Amit,
>
> On Thu, Apr 16, 2026 at 2:29 AM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Thu, Apr 16, 2026 at 5:20 PM Lakshmi N <lakshmin(dot)jhs(at)gmail(dot)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.
>

Thanks for updating the patch. Is it possible to cover this with a test case?

> Regards,
> Lakshmi
>
> [4. text/x-diff; v2-0001-Reject-pg_get_database_ddl-for-invalid-databases.patch]...

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.