public inbox for [email protected]  
help / color / mirror / Atom feed
From: Japin Li <[email protected]>
To: Lakshmi N <[email protected]>
Cc: Amit Langote <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected]
Subject: Re: Reject invalid databases in pg_get_database_ddl()
Date: Fri, 17 Apr 2026 09:40:07 +0800
Message-ID: <SY7PR01MB10921286D19E24F58EB4C24B5B6202@SY7PR01MB10921.ausprd01.prod.outlook.com> (raw)
In-Reply-To: <CA+3i_M8Mq7fr6mUspSa2rLik9+oY4sOeD1qsB_-dvnUB4NrYbg@mail.gmail.com>
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>


Hi, Lakshmi

On Thu, 16 Apr 2026 at 09:46, Lakshmi N <[email protected]> wrote:
> Hi Amit,
>
> On Thu, Apr 16, 2026 at 2:29 AM Amit Langote <[email protected]> wrote:
>
>  Hi,
>
>  On Thu, Apr 16, 2026 at 5:20 PM Lakshmi N <[email protected]> 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.





view thread (13+ messages)  latest in thread

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], [email protected], [email protected], [email protected]
  Subject: Re: Reject invalid databases in pg_get_database_ddl()
  In-Reply-To: <SY7PR01MB10921286D19E24F58EB4C24B5B6202@SY7PR01MB10921.ausprd01.prod.outlook.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