| From: | Amit Langote <amitlangote09(at)gmail(dot)com> |
|---|---|
| To: | Lakshmi N <lakshmin(dot)jhs(at)gmail(dot)com> |
| Cc: | "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, andrew(at)dunslane(dot)net |
| Subject: | Re: Reject invalid databases in pg_get_database_ddl() |
| Date: | 2026-04-16 09:28:47 |
| Message-ID: | CA+HiwqH+0rgMNQDog0AT9dVt0CGjXza_Li80njDhynWqzUwvZw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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.
*/
--
Thanks, Amit Langote
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Heikki Linnakangas | 2026-04-16 09:49:38 | Re: Reduce build times of pg_trgm GIN indexes |
| Previous Message | Yuchen Li | 2026-04-16 09:17:24 | Re: docs: Fix format of CREATE FOREIGN TABLE example in postgres_fdw |