public inbox for [email protected]
help / color / mirror / Atom feedFrom: Chao Li <[email protected]>
To: Andrew Dunstan <[email protected]>
Cc: SATYANARAYANA NARLAPURAM <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Cc: Japin Li <[email protected]>
Subject: Re: Fix a server crash problem from pg_get_database_ddl
Date: Mon, 27 Apr 2026 10:02:59 +0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<SY7PR01MB109214566B069E9C9084590FEB6232@SY7PR01MB10921.ausprd01.prod.outlook.com>
<[email protected]>
<CAHg+QDcNyJ94cCD+9ZRfz==hDnghjE5BaR4+BiSWXt82hpgDtA@mail.gmail.com>
<[email protected]>
> On Apr 26, 2026, at 22:50, Andrew Dunstan <[email protected]> wrote:
>
>
> On 2026-04-23 Th 2:47 AM, SATYANARAYANA NARLAPURAM wrote:
>>
>>
>> Thanks for printing out that. Yes, they are similar.
>>
>> I agree with what Tom said in [2]:
>> ```
>> This is not a bug. This is a superuser intentionally breaking
>> the system by corrupting the catalogs. There are any number
>> of ways to cause trouble with ill-advised manual updates to a
>> catalog table. Try, eg, "DELETE FROM pg_proc" (... but not in
>> a database you care about).
>> ```
>>
>> So, let me take back this patch.
>>
>> [2] https://www.postgresql.org/message-id/[email protected]
>> In this case, it is a very corner case but not something superuser intentionally breaks.
>> For example, a concurrent tablespace drop + database ddl to assign a different tablespace or default.
>> We aren't acquiring Access Share lock on the DB in this function (intentional) so it is a good practice
>> to do the null checks. Of course, it makes more sense to add this comment while doing a code review.
>> I will let Tom and others chime in with their thoughts on fixing this.
>>
>> Attached an injection point test to show the race. Not intended to commit.
>>
>>
>
> I agree if there's a race condition we should protect against it. I don't much like the idea of silently ignoring it, though. Raising an error seems more like the right thing to do.
>
> cheers
>
> andrew
> --
> Andrew Dunstan
> EDB: https://www.enterprisedb.com
>
The v1 patch raises an error when the tablespace name is NULL.
PFA v2: removed hint from the error message, because I now consider the hint might not be necessary.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
[application/octet-stream] v2-0001-ddlutils-error-out-when-pg_get_database_ddl-sees-.patch (1.4K, 2-v2-0001-ddlutils-error-out-when-pg_get_database_ddl-sees-.patch)
download | inline diff:
From 9e14a21e5cd5dcd7680a2cbdcc761126c8a23f79 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Wed, 15 Apr 2026 13:32:12 +0800
Subject: [PATCH v2] ddlutils: error out when pg_get_database_ddl() sees a
missing tablespace
pg_get_database_ddl_internal() calls get_tablespace_name() for a
database's dattablespace, and then passes the result to
pg_strcasecmp() without checking for NULL first.
Fix that by detecting the missing tablespace explicitly and raising an
ERROR with ERRCODE_UNDEFINED_OBJECT.
Author: Chao Li <[email protected]>
Reviewed-by:
Discussion: https://postgr.es/m/[email protected]
---
src/backend/utils/adt/ddlutils.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/backend/utils/adt/ddlutils.c b/src/backend/utils/adt/ddlutils.c
index d83cda3342e..b3f819271ad 100644
--- a/src/backend/utils/adt/ddlutils.c
+++ b/src/backend/utils/adt/ddlutils.c
@@ -986,6 +986,12 @@ pg_get_database_ddl_internal(Oid dbid, bool pretty,
{
char *spcname = get_tablespace_name(dbform->dattablespace);
+ if (spcname == NULL)
+ ereport(ERROR,
+ errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("tablespace with OID %u does not exist",
+ dbform->dattablespace));
+
if (pg_strcasecmp(spcname, "pg_default") != 0)
append_ddl_option(&buf, pretty, 4, "TABLESPACE = %s",
quote_identifier(spcname));
--
2.50.1 (Apple Git-155)
view thread (10+ 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: Fix a server crash problem from pg_get_database_ddl
In-Reply-To: <[email protected]>
* 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