public inbox for [email protected]help / color / mirror / Atom feed
Reject invalid databases in pg_get_database_ddl() 13+ messages / 6 participants [nested] [flat]
* Reject invalid databases in pg_get_database_ddl() @ 2026-04-16 08:19 Lakshmi N <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Lakshmi N @ 2026-04-16 08:19 UTC (permalink / raw) To: [email protected] <[email protected]>; +Cc: [email protected] Hi, 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(). Regards, Lakshmi Attachments: [application/octet-stream] 0001-Reject-pg_get_database_ddl-for-invalid-databases.patch (929B, 3-0001-Reject-pg_get_database_ddl-for-invalid-databases.patch) download | inline diff: diff --git a/src/backend/utils/adt/ddlutils.c b/src/backend/utils/adt/ddlutils.c index c4f9f86c43e..7ffe231417a 100644 --- a/src/backend/utils/adt/ddlutils.c +++ b/src/backend/utils/adt/ddlutils.c @@ -887,6 +887,17 @@ pg_get_database_ddl_internal(Oid dbid, bool pretty, dbform = (Form_pg_database) GETSTRUCT(tuple); dbname = pstrdup(NameStr(dbform->datname)); + /* + * Reject invalid databases. This is a safety measure to avoid generating DDL + * that can't be executed. For example, invalid connection limit. + */ + if (database_is_invalid_form(dbform)) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot generate DDL for invalid database \"%s\"", + dbname), + errhint("Use DROP DATABASE to drop invalid databases."))); + /* * We don't support generating DDL for system databases. The primary * reason for this is that users shouldn't be recreating them. ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Reject invalid databases in pg_get_database_ddl() @ 2026-04-16 09:28 Amit Langote <[email protected]> parent: Lakshmi N <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Amit Langote @ 2026-04-16 09:28 UTC (permalink / raw) To: Lakshmi N <[email protected]>; +Cc: [email protected] <[email protected]>; [email protected] 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. */ -- Thanks, Amit Langote ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Reject invalid databases in pg_get_database_ddl() @ 2026-04-16 16:46 Lakshmi N <[email protected]> parent: Amit Langote <[email protected]> 0 siblings, 2 replies; 13+ messages in thread From: Lakshmi N @ 2026-04-16 16:46 UTC (permalink / raw) To: Amit Langote <[email protected]>; +Cc: [email protected] <[email protected]>; [email protected] 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. Regards, Lakshmi Attachments: [application/octet-stream] v2-0001-Reject-pg_get_database_ddl-for-invalid-databases.patch (901B, 3-v2-0001-Reject-pg_get_database_ddl-for-invalid-databases.patch) download | inline diff: diff --git a/src/backend/utils/adt/ddlutils.c b/src/backend/utils/adt/ddlutils.c index c4f9f86c43e..533fc8c9f9c 100644 --- a/src/backend/utils/adt/ddlutils.c +++ b/src/backend/utils/adt/ddlutils.c @@ -887,6 +887,17 @@ pg_get_database_ddl_internal(Oid dbid, bool pretty, dbform = (Form_pg_database) GETSTRUCT(tuple); dbname = pstrdup(NameStr(dbform->datname)); + /* + * Reject invalid databases: datconnlimit = -2 would be emitted as + * CONNECTION LIMIT = -2, which fails on replay. + */ + if (database_is_invalid_form(dbform)) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot generate DDL for invalid database \"%s\"", + dbname), + errhint("Use DROP DATABASE to drop invalid databases."))); + /* * We don't support generating DDL for system databases. The primary * reason for this is that users shouldn't be recreating them. ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Reject invalid databases in pg_get_database_ddl() @ 2026-04-16 23:46 Amit Langote <[email protected]> parent: Lakshmi N <[email protected]> 1 sibling, 1 reply; 13+ messages in thread From: Amit Langote @ 2026-04-16 23:46 UTC (permalink / raw) To: Lakshmi N <[email protected]>; +Cc: [email protected] <[email protected]>; [email protected] On Fri, Apr 17, 2026 at 1:46 AM Lakshmi N <[email protected]> wrote: > 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. Will push the attached shortly. -- Thanks, Amit Langote Attachments: [application/octet-stream] v2-0001-Reject-invalid-databases-in-pg_get_database_ddl.patch (1.6K, 2-v2-0001-Reject-invalid-databases-in-pg_get_database_ddl.patch) download | inline diff: From ca98087734df0cddefd9610b7c23be8d1b758969 Mon Sep 17 00:00:00 2001 From: Amit Langote <[email protected]> Date: Fri, 17 Apr 2026 08:42:19 +0900 Subject: [PATCH v2] Reject invalid databases in pg_get_database_ddl() An invalid database has datconnlimit set to -2. pg_get_database_ddl() emits this verbatim as CONNECTION LIMIT = -2, which ALTER DATABASE rejects. Error out early instead. Reported-by: Lakshmi N <[email protected]> Author: Lakshmi N <[email protected]> Reviewed-by: Amit Langote <[email protected]> Discussion: https://postgr.es/m/CA+3i_M8m1k2gFch+tU0JmAQh9FRV+pFrfTXDrJo+BqmwsTmOhg@mail.gmail.com --- src/backend/utils/adt/ddlutils.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/backend/utils/adt/ddlutils.c b/src/backend/utils/adt/ddlutils.c index c4f9f86c43e..3921ed1fa6a 100644 --- a/src/backend/utils/adt/ddlutils.c +++ b/src/backend/utils/adt/ddlutils.c @@ -887,6 +887,17 @@ pg_get_database_ddl_internal(Oid dbid, bool pretty, dbform = (Form_pg_database) GETSTRUCT(tuple); dbname = pstrdup(NameStr(dbform->datname)); + /* + * Reject invalid databases: datconnlimit = -2 would be emitted as + * CONNECTION LIMIT = -2, which cannot be executed. + */ + if (database_is_invalid_form(dbform)) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot generate DDL for invalid database \"%s\"", + dbname), + errhint("Use DROP DATABASE to drop invalid databases."))); + /* * We don't support generating DDL for system databases. The primary * reason for this is that users shouldn't be recreating them. -- 2.47.3 ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Reject invalid databases in pg_get_database_ddl() @ 2026-04-17 01:40 Japin Li <[email protected]> parent: Lakshmi N <[email protected]> 1 sibling, 0 replies; 13+ messages in thread From: Japin Li @ 2026-04-17 01:40 UTC (permalink / raw) To: Lakshmi N <[email protected]>; +Cc: Amit Langote <[email protected]>; [email protected] <[email protected]>; [email protected] 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. ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Reject invalid databases in pg_get_database_ddl() @ 2026-04-17 01:46 Euler Taveira <[email protected]> parent: Amit Langote <[email protected]> 0 siblings, 2 replies; 13+ messages in thread From: Euler Taveira @ 2026-04-17 01:46 UTC (permalink / raw) To: Amit Langote <[email protected]>; Lakshmi N <[email protected]>; +Cc: [email protected] <[email protected]>; Andrew Dunstan <[email protected]> On Thu, Apr 16, 2026, at 8:46 PM, Amit Langote wrote: > > Thanks. Will push the attached shortly. > I think the errhint() is excessive in this context. It makes sense if you are executing ALTER DATABASE, for example. I suggest a message like database \"%s\" is an invalid database Regarding the test case suggested by Japin Li, I don't think it is worth because it is a transient state (unless something bad happened and pg_database contains a dangling row.) -- Euler Taveira EDB https://www.enterprisedb.com/ ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Reject invalid databases in pg_get_database_ddl() @ 2026-04-17 02:15 Amit Langote <[email protected]> parent: Euler Taveira <[email protected]> 1 sibling, 1 reply; 13+ messages in thread From: Amit Langote @ 2026-04-17 02:15 UTC (permalink / raw) To: Euler Taveira <[email protected]>; +Cc: Lakshmi N <[email protected]>; [email protected] <[email protected]>; Andrew Dunstan <[email protected]> On Fri, Apr 17, 2026 at 10:47 AM Euler Taveira <[email protected]> wrote: > On Thu, Apr 16, 2026, at 8:46 PM, Amit Langote wrote: > > > > Thanks. Will push the attached shortly. > > I think the errhint() is excessive in this context. It makes sense if you are > executing ALTER DATABASE, for example. Yeah, agreed. > I suggest a message like > > database \"%s\" is an invalid database Or just drop it, because the errmsg already says "invalid database %s". > Regarding the test case suggested by Japin Li, I don't think it is worth because > it is a transient state (unless something bad happened and pg_database contains > a dangling row.) Agreed. Patch updated. -- Thanks, Amit Langote Attachments: [application/octet-stream] v3-0001-Reject-invalid-databases-in-pg_get_database_ddl.patch (1.6K, 2-v3-0001-Reject-invalid-databases-in-pg_get_database_ddl.patch) download | inline diff: From 3607f6050947ae986dc186243685eb1dcfc8530e Mon Sep 17 00:00:00 2001 From: Amit Langote <[email protected]> Date: Fri, 17 Apr 2026 11:11:04 +0900 Subject: [PATCH v3] Reject invalid databases in pg_get_database_ddl() An invalid database has datconnlimit set to -2. pg_get_database_ddl() emits this verbatim as CONNECTION LIMIT = -2, which ALTER DATABASE rejects. Error out early instead. Reported-by: Lakshmi N <[email protected]> Author: Lakshmi N <[email protected]> Reviewed-by: Amit Langote <[email protected]> Reviewed-by: Euler Taveira <[email protected]> Discussion: https://postgr.es/m/CA+3i_M8m1k2gFch+tU0JmAQh9FRV+pFrfTXDrJo+BqmwsTmOhg@mail.gmail.com --- src/backend/utils/adt/ddlutils.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/backend/utils/adt/ddlutils.c b/src/backend/utils/adt/ddlutils.c index c4f9f86c43e..fe38117213b 100644 --- a/src/backend/utils/adt/ddlutils.c +++ b/src/backend/utils/adt/ddlutils.c @@ -887,6 +887,16 @@ pg_get_database_ddl_internal(Oid dbid, bool pretty, dbform = (Form_pg_database) GETSTRUCT(tuple); dbname = pstrdup(NameStr(dbform->datname)); + /* + * Reject invalid databases: datconnlimit = -2 would be emitted as + * CONNECTION LIMIT = -2, which cannot be executed. + */ + if (database_is_invalid_form(dbform)) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot generate DDL for invalid database \"%s\"", + dbname))); + /* * We don't support generating DDL for system databases. The primary * reason for this is that users shouldn't be recreating them. -- 2.47.3 ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Reject invalid databases in pg_get_database_ddl() @ 2026-04-17 02:30 Japin Li <[email protected]> parent: Euler Taveira <[email protected]> 1 sibling, 0 replies; 13+ messages in thread From: Japin Li @ 2026-04-17 02:30 UTC (permalink / raw) To: Euler Taveira <[email protected]>; +Cc: Amit Langote <[email protected]>; Lakshmi N <[email protected]>; [email protected] <[email protected]>; Andrew Dunstan <[email protected]> On Thu, 16 Apr 2026 at 22:46, "Euler Taveira" <[email protected]> wrote: > On Thu, Apr 16, 2026, at 8:46 PM, Amit Langote wrote: >> >> Thanks. Will push the attached shortly. >> > > I think the errhint() is excessive in this context. It makes sense if you are > executing ALTER DATABASE, for example. I suggest a message like > > database \"%s\" is an invalid database > > Regarding the test case suggested by Japin Li, I don't think it is worth because > it is a transient state (unless something bad happened and pg_database contains > a dangling row.) > Thanks for clarifying. Got it. -- Regards, Japin Li ChengDu WenWu Information Technology Co., Ltd. ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Reject invalid databases in pg_get_database_ddl() @ 2026-04-17 02:49 Hu Xunqi <[email protected]> parent: Amit Langote <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Hu Xunqi @ 2026-04-17 02:49 UTC (permalink / raw) To: Amit Langote <[email protected]>; +Cc: Euler Taveira <[email protected]>; Lakshmi N <[email protected]>; [email protected] <[email protected]>; Andrew Dunstan <[email protected]> On Fri, Apr 17, 2026 at 10:16 AM Amit Langote <[email protected]> wrote: > On Fri, Apr 17, 2026 at 10:47 AM Euler Taveira <[email protected]> wrote: > > On Thu, Apr 16, 2026, at 8:46 PM, Amit Langote wrote: > > > > > > Thanks. Will push the attached shortly. > > > > I think the errhint() is excessive in this context. It makes sense if > you are > > executing ALTER DATABASE, for example. > > Yeah, agreed. > > > I suggest a message like > > > > database \"%s\" is an invalid database > > Or just drop it, because the errmsg already says "invalid database %s". > > > Regarding the test case suggested by Japin Li, I don't think it is worth > because > > it is a transient state (unless something bad happened and pg_database > contains > > a dangling row.) > > Agreed. > +1. As this is an edge case failure, it’s not worth extending test time. > > Patch updated. > > -- > Thanks, Amit Langote > + /* + * Reject invalid databases: datconnlimit = -2 would be emitted as + * CONNECTION LIMIT = -2, which cannot be executed. + */ This comment looks a bit too centered on datconnlimit=-2, but the real issue is that an invalid pg_database row should not be deparsed into DDL. So, maybe rephrase like: /* * Reject invalid databases. Deparsing a pg_database row in invalid state * can produce SQL that is not executable, such as CONNECTION LIMIT = -2. */ Regards, Xunqi Hu ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Reject invalid databases in pg_get_database_ddl() @ 2026-04-17 03:30 Amit Langote <[email protected]> parent: Hu Xunqi <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Amit Langote @ 2026-04-17 03:30 UTC (permalink / raw) To: Hu Xunqi <[email protected]>; +Cc: Euler Taveira <[email protected]>; Lakshmi N <[email protected]>; [email protected] <[email protected]>; Andrew Dunstan <[email protected]> Hi, On Fri, Apr 17, 2026 at 11:49 AM Hu Xunqi <[email protected]> wrote: > On Fri, Apr 17, 2026 at 10:16 AM Amit Langote <[email protected]> wrote: > + /* > + * Reject invalid databases: datconnlimit = -2 would be emitted as > + * CONNECTION LIMIT = -2, which cannot be executed. > + */ > > This comment looks a bit too centered on datconnlimit=-2, but the real issue is that an invalid pg_database row should not be deparsed into DDL. So, maybe rephrase like: > > /* > * Reject invalid databases. Deparsing a pg_database row in invalid state > * can produce SQL that is not executable, such as CONNECTION LIMIT = -2. > */ I was trying to be precise about datconnlimit = -2 being the thing that produces invalid SQL. But your version covers that with the "such as CONNECTION LIMIT = -2" example, and it's closer to the original, which was on the right track, just needed to be more precise. Let's go with it. -- Thanks, Amit Langote Attachments: [application/octet-stream] v4-0001-Reject-invalid-databases-in-pg_get_database_ddl.patch (1.7K, 2-v4-0001-Reject-invalid-databases-in-pg_get_database_ddl.patch) download | inline diff: From 3d8899093a0f07e023711c96c4741d6fb40ecba9 Mon Sep 17 00:00:00 2001 From: Amit Langote <[email protected]> Date: Fri, 17 Apr 2026 12:26:22 +0900 Subject: [PATCH v4] Reject invalid databases in pg_get_database_ddl() An invalid database has datconnlimit set to -2. pg_get_database_ddl() emits this verbatim as CONNECTION LIMIT = -2, which ALTER DATABASE rejects. Error out early instead. Reported-by: Lakshmi N <[email protected]> Author: Lakshmi N <[email protected]> Reviewed-by: Amit Langote <[email protected]> Reviewed-by: Euler Taveira <[email protected]> Reviewed-by: Hu Xunqi <[email protected]> Discussion: https://postgr.es/m/CA+3i_M8m1k2gFch+tU0JmAQh9FRV+pFrfTXDrJo+BqmwsTmOhg@mail.gmail.com --- src/backend/utils/adt/ddlutils.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/backend/utils/adt/ddlutils.c b/src/backend/utils/adt/ddlutils.c index c4f9f86c43e..d83cda3342e 100644 --- a/src/backend/utils/adt/ddlutils.c +++ b/src/backend/utils/adt/ddlutils.c @@ -887,6 +887,16 @@ pg_get_database_ddl_internal(Oid dbid, bool pretty, dbform = (Form_pg_database) GETSTRUCT(tuple); dbname = pstrdup(NameStr(dbform->datname)); + /* + * Reject invalid databases. Deparsing a pg_database row in invalid state + * can produce SQL that is not executable, such as CONNECTION LIMIT = -2. + */ + if (database_is_invalid_form(dbform)) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot generate DDL for invalid database \"%s\"", + dbname))); + /* * We don't support generating DDL for system databases. The primary * reason for this is that users shouldn't be recreating them. -- 2.47.3 ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Reject invalid databases in pg_get_database_ddl() @ 2026-04-17 04:20 Lakshmi N <[email protected]> parent: Amit Langote <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Lakshmi N @ 2026-04-17 04:20 UTC (permalink / raw) To: Amit Langote <[email protected]>; +Cc: Hu Xunqi <[email protected]>; Euler Taveira <[email protected]>; [email protected] <[email protected]>; Andrew Dunstan <[email protected]> Hi, On Thu, Apr 16, 2026 at 8:31 PM Amit Langote <[email protected]> wrote: > Hi, > > On Fri, Apr 17, 2026 at 11:49 AM Hu Xunqi <[email protected]> wrote: > > On Fri, Apr 17, 2026 at 10:16 AM Amit Langote <[email protected]> > wrote: > > + /* > > + * Reject invalid databases: datconnlimit = -2 would be emitted > as > > + * CONNECTION LIMIT = -2, which cannot be executed. > > + */ > > > > This comment looks a bit too centered on datconnlimit=-2, but the real > issue is that an invalid pg_database row should not be deparsed into DDL. > So, maybe rephrase like: > > > > /* > > * Reject invalid databases. Deparsing a pg_database row in invalid state > > * can produce SQL that is not executable, such as CONNECTION LIMIT = -2. > > */ > > I was trying to be precise about datconnlimit = -2 being the thing > that produces invalid SQL. But your version covers that with the "such > as CONNECTION LIMIT = -2" example, and it's closer to the original, > which was on the right track, just needed to be more precise. Let's go > with it. > This looks good to me. Thank you for reviewing and making the changes! Regards, Lakshmi ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Reject invalid databases in pg_get_database_ddl() @ 2026-04-17 04:22 Amit Langote <[email protected]> parent: Lakshmi N <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Amit Langote @ 2026-04-17 04:22 UTC (permalink / raw) To: Lakshmi N <[email protected]>; +Cc: Hu Xunqi <[email protected]>; Euler Taveira <[email protected]>; [email protected] <[email protected]>; Andrew Dunstan <[email protected]> On Fri, Apr 17, 2026 at 1:21 PM Lakshmi N <[email protected]> wrote: > On Thu, Apr 16, 2026 at 8:31 PM Amit Langote <[email protected]> wrote: >> On Fri, Apr 17, 2026 at 11:49 AM Hu Xunqi <[email protected]> wrote: >> > On Fri, Apr 17, 2026 at 10:16 AM Amit Langote <[email protected]> wrote: >> > + /* >> > + * Reject invalid databases: datconnlimit = -2 would be emitted as >> > + * CONNECTION LIMIT = -2, which cannot be executed. >> > + */ >> > >> > This comment looks a bit too centered on datconnlimit=-2, but the real issue is that an invalid pg_database row should not be deparsed into DDL. So, maybe rephrase like: >> > >> > /* >> > * Reject invalid databases. Deparsing a pg_database row in invalid state >> > * can produce SQL that is not executable, such as CONNECTION LIMIT = -2. >> > */ >> >> I was trying to be precise about datconnlimit = -2 being the thing >> that produces invalid SQL. But your version covers that with the "such >> as CONNECTION LIMIT = -2" example, and it's closer to the original, >> which was on the right track, just needed to be more precise. Let's go >> with it. > > This looks good to me. Thank you for reviewing and making the changes! Pushed. -- Thanks, Amit Langote ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Reject invalid databases in pg_get_database_ddl() @ 2026-04-17 12:40 Andrew Dunstan <[email protected]> parent: Amit Langote <[email protected]> 0 siblings, 0 replies; 13+ messages in thread From: Andrew Dunstan @ 2026-04-17 12:40 UTC (permalink / raw) To: Amit Langote <[email protected]>; Lakshmi N <[email protected]>; +Cc: Hu Xunqi <[email protected]>; Euler Taveira <[email protected]>; [email protected] <[email protected]> On 2026-04-17 Fr 12:22 AM, Amit Langote wrote: > On Fri, Apr 17, 2026 at 1:21 PM Lakshmi N <[email protected]> wrote: >> On Thu, Apr 16, 2026 at 8:31 PM Amit Langote <[email protected]> wrote: >>> On Fri, Apr 17, 2026 at 11:49 AM Hu Xunqi <[email protected]> wrote: >>>> On Fri, Apr 17, 2026 at 10:16 AM Amit Langote <[email protected]> wrote: >>>> + /* >>>> + * Reject invalid databases: datconnlimit = -2 would be emitted as >>>> + * CONNECTION LIMIT = -2, which cannot be executed. >>>> + */ >>>> >>>> This comment looks a bit too centered on datconnlimit=-2, but the real issue is that an invalid pg_database row should not be deparsed into DDL. So, maybe rephrase like: >>>> >>>> /* >>>> * Reject invalid databases. Deparsing a pg_database row in invalid state >>>> * can produce SQL that is not executable, such as CONNECTION LIMIT = -2. >>>> */ >>> I was trying to be precise about datconnlimit = -2 being the thing >>> that produces invalid SQL. But your version covers that with the "such >>> as CONNECTION LIMIT = -2" example, and it's closer to the original, >>> which was on the right track, just needed to be more precise. Let's go >>> with it. >> This looks good to me. Thank you for reviewing and making the changes! > Pushed. > Thanks for jumping on this. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com ^ permalink raw reply [nested|flat] 13+ messages in thread
end of thread, other threads:[~2026-04-17 12:40 UTC | newest] Thread overview: 13+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-04-16 08:19 Reject invalid databases in pg_get_database_ddl() Lakshmi N <[email protected]> 2026-04-16 09:28 ` Amit Langote <[email protected]> 2026-04-16 16:46 ` Lakshmi N <[email protected]> 2026-04-16 23:46 ` Amit Langote <[email protected]> 2026-04-17 01:46 ` Euler Taveira <[email protected]> 2026-04-17 02:15 ` Amit Langote <[email protected]> 2026-04-17 02:49 ` Hu Xunqi <[email protected]> 2026-04-17 03:30 ` Amit Langote <[email protected]> 2026-04-17 04:20 ` Lakshmi N <[email protected]> 2026-04-17 04:22 ` Amit Langote <[email protected]> 2026-04-17 12:40 ` Andrew Dunstan <[email protected]> 2026-04-17 02:30 ` Japin Li <[email protected]> 2026-04-17 01:40 ` Japin Li <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox