public inbox for [email protected]
help / color / mirror / Atom feedFix a server crash problem from pg_get_database_ddl
10+ messages / 5 participants
[nested] [flat]
* Fix a server crash problem from pg_get_database_ddl
@ 2026-04-15 05:51 Chao Li <[email protected]>
2026-04-16 00:44 ` Re: Fix a server crash problem from pg_get_database_ddl Jack Bonatakis <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Chao Li @ 2026-04-15 05:51 UTC (permalink / raw)
To: Postgres hackers <[email protected]>
Hi,
While doing some testing, I hit a server crash:
```
2026-04-15 11:30:17.377 CST [98179] LOG: client backend (PID 41260) was terminated by signal 11: Segmentation fault: 11
2026-04-15 11:30:17.377 CST [98179] DETAIL: Failed process was running: SELECT * FROM pg_get_database_ddl('db1'::regdatabase);
2026-04-15 11:30:17.377 CST [98179] LOG: terminating any other active server processes
2026-04-15 11:30:17.380 CST [44361] FATAL: the database system is in recovery mode
```
After debugging it, I found that the crash happened because I had mistakenly deleted the tablespace entry directly from pg_tablespace, and pg_get_database_ddl_internal() calls get_tablespace_name() without checking whether the return value is NULL.
So this doesn't seem like a bug a normal user could hit. It is more like a superuser-only mistake that creates an invalid catalog state. I think that even in such an edge case, we should raise a proper error instead of crashing the backend.
BTW, I have verified that in this case, ALTER DATABASE ... SET TABLESPACE can move the database to a valid tablespace and recover from the issue.
This patch fixes that by checking for a NULL result and throwing an error.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
[application/octet-stream] v1-0001-ddlutils-error-out-when-pg_get_database_ddl-sees-.patch (1.4K, 2-v1-0001-ddlutils-error-out-when-pg_get_database_ddl-sees-.patch)
download | inline diff:
From 4529c77c01bd0a2f1718c6dc45a2896191b72001 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Wed, 15 Apr 2026 13:32:12 +0800
Subject: [PATCH v1] 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/
---
src/backend/utils/adt/ddlutils.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/backend/utils/adt/ddlutils.c b/src/backend/utils/adt/ddlutils.c
index c4f9f86c43e..6ab354e42f6 100644
--- a/src/backend/utils/adt/ddlutils.c
+++ b/src/backend/utils/adt/ddlutils.c
@@ -976,6 +976,13 @@ 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),
+ errhint("To recover, try ALTER DATABASE ... SET TABLESPACE ... to a valid tablespace."));
+
if (pg_strcasecmp(spcname, "pg_default") != 0)
append_ddl_option(&buf, pretty, 4, "TABLESPACE = %s",
quote_identifier(spcname));
--
2.50.1 (Apple Git-155)
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fix a server crash problem from pg_get_database_ddl
2026-04-15 05:51 Fix a server crash problem from pg_get_database_ddl Chao Li <[email protected]>
@ 2026-04-16 00:44 ` Jack Bonatakis <[email protected]>
2026-04-16 01:23 ` Re: Fix a server crash problem from pg_get_database_ddl Japin Li <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Jack Bonatakis @ 2026-04-16 00:44 UTC (permalink / raw)
To: [email protected]
I have reproduced this error against the current master:
```
CREATE TABLESPACE ts1 LOCATION '/workspace/tablespaces/pg_bug_ts1';
CREATE DATABASE db1 TABLESPACE ts1;
DELETE FROM pg_tablespace WHERE spcname = 'ts1';
SELECT * FROM pg_get_database_ddl('db1'::regdatabase);
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
```
Backend logs show:
```
[1] LOG: client backend (PID 15420) was terminated by signal 11: Segmentation fault
[1] DETAIL: Failed process was running: SELECT * FROM pg_get_database_ddl('db1'::regdatabase);
[1] LOG: terminating any other active server processes
```
After applying the patch:
```
SELECT * FROM pg_get_database_ddl('db1'::regdatabase);
ERROR: tablespace with OID 16393 does not exist
HINT: To recover, try ALTER DATABASE ... SET TABLESPACE ... to a valid tablespace.
```
and backend logs show:
```
[56] ERROR: tablespace with OID 16393 does not exist
[56] HINT: To recover, try ALTER DATABASE ... SET TABLESPACE ... to a valid tablespace.
[56] STATEMENT: SELECT * FROM pg_get_database_ddl('db1'::regdatabase);
```
All tests pass.
The only note I'd have on the code change is that there is no accompanying test. It seems like a TAP test would be reasonable, but I am quite new and will defer to whether you think that's the right call or even necessary.
Jack
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fix a server crash problem from pg_get_database_ddl
2026-04-15 05:51 Fix a server crash problem from pg_get_database_ddl Chao Li <[email protected]>
2026-04-16 00:44 ` Re: Fix a server crash problem from pg_get_database_ddl Jack Bonatakis <[email protected]>
@ 2026-04-16 01:23 ` Japin Li <[email protected]>
2026-04-16 01:35 ` Re: Fix a server crash problem from pg_get_database_ddl Chao Li <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Japin Li @ 2026-04-16 01:23 UTC (permalink / raw)
To: Jack Bonatakis <[email protected]>; +Cc: [email protected]
On Wed, 15 Apr 2026 at 20:44, "Jack Bonatakis" <[email protected]> wrote:
> I have reproduced this error against the current master:
>
> ```
> CREATE TABLESPACE ts1 LOCATION '/workspace/tablespaces/pg_bug_ts1';
> CREATE DATABASE db1 TABLESPACE ts1;
> DELETE FROM pg_tablespace WHERE spcname = 'ts1';
> SELECT * FROM pg_get_database_ddl('db1'::regdatabase);
>
> server closed the connection unexpectedly
> This probably means the server terminated abnormally
> before or while processing the request.
> The connection to the server was lost. Attempting reset: Failed.
> ```
> Backend logs show:
>
> ```
> [1] LOG: client backend (PID 15420) was terminated by signal 11: Segmentation fault
> [1] DETAIL: Failed process was running: SELECT * FROM pg_get_database_ddl('db1'::regdatabase);
> [1] LOG: terminating any other active server processes
> ```
> After applying the patch:
>
> ```
> SELECT * FROM pg_get_database_ddl('db1'::regdatabase);
> ERROR: tablespace with OID 16393 does not exist
> HINT: To recover, try ALTER DATABASE ... SET TABLESPACE ... to a valid tablespace.
> ```
> and backend logs show:
>
> ```
> [56] ERROR: tablespace with OID 16393 does not exist
> [56] HINT: To recover, try ALTER DATABASE ... SET TABLESPACE ... to a valid tablespace.
> [56] STATEMENT: SELECT * FROM pg_get_database_ddl('db1'::regdatabase);
> ```
> All tests pass.
>
> The only note I'd have on the code change is that there is no accompanying test. It seems like a TAP test would be
> reasonable, but I am quite new and will defer to whether you think that's the right call or even necessary.
>
> Jack
This seems similar to [1]. Could you please confirm?
[1] https://www.postgresql.org/message-id/CAJTYsWXcd324VELk%3D9KdsfTsua9So3Yexqv7N3B23h9zAUD40g%40mail.g....
--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fix a server crash problem from pg_get_database_ddl
2026-04-15 05:51 Fix a server crash problem from pg_get_database_ddl Chao Li <[email protected]>
2026-04-16 00:44 ` Re: Fix a server crash problem from pg_get_database_ddl Jack Bonatakis <[email protected]>
2026-04-16 01:23 ` Re: Fix a server crash problem from pg_get_database_ddl Japin Li <[email protected]>
@ 2026-04-16 01:35 ` Chao Li <[email protected]>
2026-04-23 06:47 ` Re: Fix a server crash problem from pg_get_database_ddl SATYANARAYANA NARLAPURAM <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Chao Li @ 2026-04-16 01:35 UTC (permalink / raw)
To: Japin Li <[email protected]>; +Cc: Jack Bonatakis <[email protected]>; [email protected]
> On Apr 16, 2026, at 09:23, Japin Li <[email protected]> wrote:
>
> On Wed, 15 Apr 2026 at 20:44, "Jack Bonatakis" <[email protected]> wrote:
>> I have reproduced this error against the current master:
>>
>> ```
>> CREATE TABLESPACE ts1 LOCATION '/workspace/tablespaces/pg_bug_ts1';
>> CREATE DATABASE db1 TABLESPACE ts1;
>> DELETE FROM pg_tablespace WHERE spcname = 'ts1';
>> SELECT * FROM pg_get_database_ddl('db1'::regdatabase);
>>
>> server closed the connection unexpectedly
>> This probably means the server terminated abnormally
>> before or while processing the request.
>> The connection to the server was lost. Attempting reset: Failed.
>> ```
>> Backend logs show:
>>
>> ```
>> [1] LOG: client backend (PID 15420) was terminated by signal 11: Segmentation fault
>> [1] DETAIL: Failed process was running: SELECT * FROM pg_get_database_ddl('db1'::regdatabase);
>> [1] LOG: terminating any other active server processes
>> ```
>> After applying the patch:
>>
>> ```
>> SELECT * FROM pg_get_database_ddl('db1'::regdatabase);
>> ERROR: tablespace with OID 16393 does not exist
>> HINT: To recover, try ALTER DATABASE ... SET TABLESPACE ... to a valid tablespace.
>> ```
>> and backend logs show:
>>
>> ```
>> [56] ERROR: tablespace with OID 16393 does not exist
>> [56] HINT: To recover, try ALTER DATABASE ... SET TABLESPACE ... to a valid tablespace.
>> [56] STATEMENT: SELECT * FROM pg_get_database_ddl('db1'::regdatabase);
>> ```
>> All tests pass.
>>
>> The only note I'd have on the code change is that there is no accompanying test. It seems like a TAP test would be
>> reasonable, but I am quite new and will defer to whether you think that's the right call or even necessary.
>>
>> Jack
>
> This seems similar to [1]. Could you please confirm?
>
> [1] https://www.postgresql.org/message-id/CAJTYsWXcd324VELk%3D9KdsfTsua9So3Yexqv7N3B23h9zAUD40g%40mail.g....
>
> --
> Regards,
> Japin Li
> ChengDu WenWu Information Technology Co., Ltd.
>
>
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]
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fix a server crash problem from pg_get_database_ddl
2026-04-15 05:51 Fix a server crash problem from pg_get_database_ddl Chao Li <[email protected]>
2026-04-16 00:44 ` Re: Fix a server crash problem from pg_get_database_ddl Jack Bonatakis <[email protected]>
2026-04-16 01:23 ` Re: Fix a server crash problem from pg_get_database_ddl Japin Li <[email protected]>
2026-04-16 01:35 ` Re: Fix a server crash problem from pg_get_database_ddl Chao Li <[email protected]>
@ 2026-04-23 06:47 ` SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-26 14:50 ` Re: Fix a server crash problem from pg_get_database_ddl Andrew Dunstan <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: SATYANARAYANA NARLAPURAM @ 2026-04-23 06:47 UTC (permalink / raw)
To: Chao Li <[email protected]>; Tom Lane <[email protected]>; +Cc: Japin Li <[email protected]>; Jack Bonatakis <[email protected]>; [email protected]
Hi,
Adding Tom to the thread explicitly to seek his opinion.
On Wed, Apr 15, 2026 at 6:36 PM Chao Li <[email protected]> wrote:
>
>
> > On Apr 16, 2026, at 09:23, Japin Li <[email protected]> wrote:
> >
> > On Wed, 15 Apr 2026 at 20:44, "Jack Bonatakis" <[email protected]> wrote:
> >> I have reproduced this error against the current master:
> >>
> >> ```
> >> CREATE TABLESPACE ts1 LOCATION '/workspace/tablespaces/pg_bug_ts1';
> >> CREATE DATABASE db1 TABLESPACE ts1;
> >> DELETE FROM pg_tablespace WHERE spcname = 'ts1';
> >> SELECT * FROM pg_get_database_ddl('db1'::regdatabase);
> >>
> >> server closed the connection unexpectedly
> >> This probably means the server terminated abnormally
> >> before or while processing the request.
> >> The connection to the server was lost. Attempting reset: Failed.
> >> ```
> >> Backend logs show:
> >>
> >> ```
> >> [1] LOG: client backend (PID 15420) was terminated by signal 11:
> Segmentation fault
> >> [1] DETAIL: Failed process was running: SELECT * FROM
> pg_get_database_ddl('db1'::regdatabase);
> >> [1] LOG: terminating any other active server processes
> >> ```
> >> After applying the patch:
> >>
> >> ```
> >> SELECT * FROM pg_get_database_ddl('db1'::regdatabase);
> >> ERROR: tablespace with OID 16393 does not exist
> >> HINT: To recover, try ALTER DATABASE ... SET TABLESPACE ... to a valid
> tablespace.
> >> ```
> >> and backend logs show:
> >>
> >> ```
> >> [56] ERROR: tablespace with OID 16393 does not exist
> >> [56] HINT: To recover, try ALTER DATABASE ... SET TABLESPACE ... to a
> valid tablespace.
> >> [56] STATEMENT: SELECT * FROM pg_get_database_ddl('db1'::regdatabase);
> >> ```
> >> All tests pass.
> >>
> >> The only note I'd have on the code change is that there is no
> accompanying test. It seems like a TAP test would be
> >> reasonable, but I am quite new and will defer to whether you think
> that's the right call or even necessary.
> >>
> >> Jack
> >
> > This seems similar to [1]. Could you please confirm?
> >
> > [1]
> https://www.postgresql.org/message-id/CAJTYsWXcd324VELk%3D9KdsfTsua9So3Yexqv7N3B23h9zAUD40g%40mail.g...
> .
> >
> > --
> > Regards,
> > Japin Li
> > ChengDu WenWu Information Technology Co., Ltd.
> >
> >
>
> 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.
Thanks,
Satya
Attachments:
[application/octet-stream] 0002-Test-pg_get_database_ddl-race-with-concurrent-tablespace-drop.patch (5.1K, 3-0002-Test-pg_get_database_ddl-race-with-concurrent-tablespace-drop.patch)
download | inline diff:
diff --git a/src/backend/utils/adt/ddlutils.c b/src/backend/utils/adt/ddlutils.c
index d83cda33..b57e7d27 100644
--- a/src/backend/utils/adt/ddlutils.c
+++ b/src/backend/utils/adt/ddlutils.c
@@ -35,6 +35,7 @@
#include "utils/acl.h"
#include "utils/array.h"
#include "utils/builtins.h"
+#include "utils/injection_point.h"
#include "utils/datetime.h"
#include "utils/fmgroids.h"
#include "utils/guc.h"
@@ -984,9 +985,13 @@ pg_get_database_ddl_internal(Oid dbid, bool pretty,
/* TABLESPACE */
if (!no_tablespace && OidIsValid(dbform->dattablespace))
{
- char *spcname = get_tablespace_name(dbform->dattablespace);
+ char *spcname;
- if (pg_strcasecmp(spcname, "pg_default") != 0)
+ INJECTION_POINT("pg_get_database_ddl-before-get_tablespace_name", NULL);
+ spcname = get_tablespace_name(dbform->dattablespace);
+
+ if (spcname != NULL &&
+ pg_strcasecmp(spcname, "pg_default") != 0)
append_ddl_option(&buf, pretty, 4, "TABLESPACE = %s",
quote_identifier(spcname));
}
diff --git a/src/test/modules/test_misc/t/012_ddlutils_tablespace_race.pl b/src/test/modules/test_misc/t/012_ddlutils_tablespace_race.pl
new file mode 100644
index 00000000..76d31afb
--- /dev/null
+++ b/src/test/modules/test_misc/t/012_ddlutils_tablespace_race.pl
@@ -0,0 +1,111 @@
+
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# Test for race condition in pg_get_database_ddl() when a tablespace
+# is dropped concurrently.
+#
+# pg_get_database_ddl() reads the database's dattablespace OID from the
+# syscache, then later calls get_tablespace_name() which does a fresh
+# catalog scan. If the tablespace is dropped between these two operations,
+# get_tablespace_name() returns NULL and the unpatched code passes NULL
+# to pg_strcasecmp(), causing a SIGSEGV.
+
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+plan skip_all => 'Injection points not supported by this build'
+ unless $ENV{enable_injection_points} eq 'yes';
+
+# Node initialization
+my $node = PostgreSQL::Test::Cluster->new('node');
+$node->init();
+$node->append_conf('postgresql.conf',
+ "shared_preload_libraries = 'injection_points'");
+$node->start();
+
+# Check if the extension injection_points is available
+plan skip_all => 'Extension injection_points not installed'
+ unless $node->check_extension('injection_points');
+
+$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
+
+# Create tablespace and database using it
+$node->safe_psql('postgres', q[
+SET allow_in_place_tablespaces = on;
+CREATE TABLESPACE race_ts LOCATION '';
+]);
+$node->safe_psql('postgres',
+ "CREATE DATABASE race_db TABLESPACE race_ts;");
+
+# Verify setup
+my $result = $node->safe_psql('postgres',
+ "SELECT spcname FROM pg_database d JOIN pg_tablespace t "
+ . "ON d.dattablespace = t.oid WHERE datname = 'race_db';");
+is($result, 'race_ts', 'race_db is on race_ts tablespace');
+
+############################################################################
+note('Test: pg_get_database_ddl with concurrent tablespace drop');
+
+# Attach injection point that pauses before get_tablespace_name()
+$node->safe_psql('postgres',
+ "SELECT injection_points_attach("
+ . "'pg_get_database_ddl-before-get_tablespace_name', 'wait');");
+
+# Session A: call pg_get_database_ddl - will block at injection point
+my $session_a = $node->background_psql('postgres');
+
+$session_a->query_until(
+ qr//, q[
+SELECT pg_get_database_ddl('race_db'::regdatabase);
+\g
+]);
+
+# Wait for Session A to reach the injection point
+$node->wait_for_event('client backend',
+ 'pg_get_database_ddl-before-get_tablespace_name');
+
+note('Session A is paused before get_tablespace_name()');
+
+# Session B: move database off the tablespace and drop it
+$node->safe_psql('postgres',
+ "ALTER DATABASE race_db SET TABLESPACE pg_default;");
+$node->safe_psql('postgres', "DROP TABLESPACE race_ts;");
+
+# Verify the tablespace is gone
+$result = $node->safe_psql('postgres',
+ "SELECT count(*) FROM pg_tablespace WHERE spcname = 'race_ts';");
+is($result, '0', 'tablespace race_ts has been dropped');
+
+note('Waking up Session A - get_tablespace_name() will return NULL');
+
+# Wake up Session A - it will now call get_tablespace_name() with a
+# stale OID that no longer exists. Without the NULL check fix, this
+# crashes with SIGSEGV in pg_strcasecmp(NULL, "pg_default").
+$node->safe_psql('postgres',
+ "SELECT injection_points_wakeup("
+ . "'pg_get_database_ddl-before-get_tablespace_name');");
+
+# Collect result from Session A
+my $output = $session_a->query_safe(q[\g]);
+$session_a->quit();
+
+# The DDL should succeed and NOT include TABLESPACE (since the database
+# is now on pg_default)
+like($output, qr/CREATE DATABASE race_db/,
+ 'pg_get_database_ddl completed without crash');
+unlike($output, qr/TABLESPACE/,
+ 'DDL output does not include dropped tablespace');
+
+# Detach injection point
+$node->safe_psql('postgres',
+ "SELECT injection_points_detach("
+ . "'pg_get_database_ddl-before-get_tablespace_name');");
+
+# Clean up
+$node->safe_psql('postgres', "DROP DATABASE race_db;");
+
+done_testing();
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fix a server crash problem from pg_get_database_ddl
2026-04-15 05:51 Fix a server crash problem from pg_get_database_ddl Chao Li <[email protected]>
2026-04-16 00:44 ` Re: Fix a server crash problem from pg_get_database_ddl Jack Bonatakis <[email protected]>
2026-04-16 01:23 ` Re: Fix a server crash problem from pg_get_database_ddl Japin Li <[email protected]>
2026-04-16 01:35 ` Re: Fix a server crash problem from pg_get_database_ddl Chao Li <[email protected]>
2026-04-23 06:47 ` Re: Fix a server crash problem from pg_get_database_ddl SATYANARAYANA NARLAPURAM <[email protected]>
@ 2026-04-26 14:50 ` Andrew Dunstan <[email protected]>
2026-04-27 02:02 ` Re: Fix a server crash problem from pg_get_database_ddl Chao Li <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Andrew Dunstan @ 2026-04-26 14:50 UTC (permalink / raw)
To: SATYANARAYANA NARLAPURAM <[email protected]>; Tom Lane <[email protected]>; Chao Li <[email protected]>; pgsql-hackers; Japin Li <[email protected]>
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
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fix a server crash problem from pg_get_database_ddl
2026-04-15 05:51 Fix a server crash problem from pg_get_database_ddl Chao Li <[email protected]>
2026-04-16 00:44 ` Re: Fix a server crash problem from pg_get_database_ddl Jack Bonatakis <[email protected]>
2026-04-16 01:23 ` Re: Fix a server crash problem from pg_get_database_ddl Japin Li <[email protected]>
2026-04-16 01:35 ` Re: Fix a server crash problem from pg_get_database_ddl Chao Li <[email protected]>
2026-04-23 06:47 ` Re: Fix a server crash problem from pg_get_database_ddl SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-26 14:50 ` Re: Fix a server crash problem from pg_get_database_ddl Andrew Dunstan <[email protected]>
@ 2026-04-27 02:02 ` Chao Li <[email protected]>
2026-04-27 04:56 ` Re: Fix a server crash problem from pg_get_database_ddl SATYANARAYANA NARLAPURAM <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Chao Li @ 2026-04-27 02:02 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: SATYANARAYANA NARLAPURAM <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers; Japin Li <[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)
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fix a server crash problem from pg_get_database_ddl
2026-04-15 05:51 Fix a server crash problem from pg_get_database_ddl Chao Li <[email protected]>
2026-04-16 00:44 ` Re: Fix a server crash problem from pg_get_database_ddl Jack Bonatakis <[email protected]>
2026-04-16 01:23 ` Re: Fix a server crash problem from pg_get_database_ddl Japin Li <[email protected]>
2026-04-16 01:35 ` Re: Fix a server crash problem from pg_get_database_ddl Chao Li <[email protected]>
2026-04-23 06:47 ` Re: Fix a server crash problem from pg_get_database_ddl SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-26 14:50 ` Re: Fix a server crash problem from pg_get_database_ddl Andrew Dunstan <[email protected]>
2026-04-27 02:02 ` Re: Fix a server crash problem from pg_get_database_ddl Chao Li <[email protected]>
@ 2026-04-27 04:56 ` SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-30 06:40 ` Re: Fix a server crash problem from pg_get_database_ddl Chao Li <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: SATYANARAYANA NARLAPURAM @ 2026-04-27 04:56 UTC (permalink / raw)
To: Chao Li <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers; Japin Li <[email protected]>
Hi Chao,
On Sun, Apr 26, 2026 at 7:03 PM Chao Li <[email protected]> wrote:
>
>
> > 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.
>
+ if (spcname == NULL)
+ ereport(ERROR,
+ errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("tablespace with OID %u does not exist",
+ dbform->dattablespace));
+
A message with error detail that says a concurrent DDL could have dropped a
tablespace could be better?
System catalog is optional.
Something like:
errdetail("The tablespace may have been dropped concurrently, or the system
catalog is inconsistent.")));
Thanks,
Satya
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fix a server crash problem from pg_get_database_ddl
2026-04-15 05:51 Fix a server crash problem from pg_get_database_ddl Chao Li <[email protected]>
2026-04-16 00:44 ` Re: Fix a server crash problem from pg_get_database_ddl Jack Bonatakis <[email protected]>
2026-04-16 01:23 ` Re: Fix a server crash problem from pg_get_database_ddl Japin Li <[email protected]>
2026-04-16 01:35 ` Re: Fix a server crash problem from pg_get_database_ddl Chao Li <[email protected]>
2026-04-23 06:47 ` Re: Fix a server crash problem from pg_get_database_ddl SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-26 14:50 ` Re: Fix a server crash problem from pg_get_database_ddl Andrew Dunstan <[email protected]>
2026-04-27 02:02 ` Re: Fix a server crash problem from pg_get_database_ddl Chao Li <[email protected]>
2026-04-27 04:56 ` Re: Fix a server crash problem from pg_get_database_ddl SATYANARAYANA NARLAPURAM <[email protected]>
@ 2026-04-30 06:40 ` Chao Li <[email protected]>
2026-04-30 14:54 ` Re: Fix a server crash problem from pg_get_database_ddl Andrew Dunstan <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Chao Li @ 2026-04-30 06:40 UTC (permalink / raw)
To: SATYANARAYANA NARLAPURAM <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers; Japin Li <[email protected]>
> On Apr 27, 2026, at 12:56, SATYANARAYANA NARLAPURAM <[email protected]> wrote:
>
> Hi Chao,
>
> On Sun, Apr 26, 2026 at 7:03 PM Chao Li <[email protected]> wrote:
>
>
> > 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.
>
> + if (spcname == NULL)
> + ereport(ERROR,
> + errcode(ERRCODE_UNDEFINED_OBJECT),
> + errmsg("tablespace with OID %u does not exist",
> + dbform->dattablespace));
> +
>
> A message with error detail that says a concurrent DDL could have dropped a tablespace could be better?
> System catalog is optional.
> Something like:
>
> errdetail("The tablespace may have been dropped concurrently, or the system catalog is inconsistent.")));
>
> Thanks,
> Satya
Hi Satya,
Thanks for your review. I hesitate to add a detail message here because we do not actually know the root cause. A concurrent DROP TABLESPACE could be one cause, but some unusual user operation could also lead to the same result, so I am not sure the detail message would help much.
The main purpose of this patch is to avoid passing NULL to pg_strcasecmp() and to report the missing object clearly. So I think the errmsg itself is enough.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fix a server crash problem from pg_get_database_ddl
2026-04-15 05:51 Fix a server crash problem from pg_get_database_ddl Chao Li <[email protected]>
2026-04-16 00:44 ` Re: Fix a server crash problem from pg_get_database_ddl Jack Bonatakis <[email protected]>
2026-04-16 01:23 ` Re: Fix a server crash problem from pg_get_database_ddl Japin Li <[email protected]>
2026-04-16 01:35 ` Re: Fix a server crash problem from pg_get_database_ddl Chao Li <[email protected]>
2026-04-23 06:47 ` Re: Fix a server crash problem from pg_get_database_ddl SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-26 14:50 ` Re: Fix a server crash problem from pg_get_database_ddl Andrew Dunstan <[email protected]>
2026-04-27 02:02 ` Re: Fix a server crash problem from pg_get_database_ddl Chao Li <[email protected]>
2026-04-27 04:56 ` Re: Fix a server crash problem from pg_get_database_ddl SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-30 06:40 ` Re: Fix a server crash problem from pg_get_database_ddl Chao Li <[email protected]>
@ 2026-04-30 14:54 ` Andrew Dunstan <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Andrew Dunstan @ 2026-04-30 14:54 UTC (permalink / raw)
To: Chao Li <[email protected]>; +Cc: SATYANARAYANA NARLAPURAM <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers; Japin Li <[email protected]>
On Thu, Apr 30, 2026 at 2:41 AM Chao Li <[email protected]> wrote:
>
>
> > On Apr 27, 2026, at 12:56, SATYANARAYANA NARLAPURAM <
> [email protected]> wrote:
> >
> > Hi Chao,
> >
> > On Sun, Apr 26, 2026 at 7:03 PM Chao Li <[email protected]> wrote:
> >
> >
> > > 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.
> >
> > + if (spcname == NULL)
> > + ereport(ERROR,
> > + errcode(ERRCODE_UNDEFINED_OBJECT),
> > + errmsg("tablespace with OID %u does not exist",
> > + dbform->dattablespace));
> > +
> >
> > A message with error detail that says a concurrent DDL could have
> dropped a tablespace could be better?
> > System catalog is optional.
> > Something like:
> >
> > errdetail("The tablespace may have been dropped concurrently, or the
> system catalog is inconsistent.")));
> >
> > Thanks,
> > Satya
>
> Hi Satya,
>
> Thanks for your review. I hesitate to add a detail message here because we
> do not actually know the root cause. A concurrent DROP TABLESPACE could be
> one cause, but some unusual user operation could also lead to the same
> result, so I am not sure the detail message would help much.
>
> The main purpose of this patch is to avoid passing NULL to pg_strcasecmp()
> and to report the missing object clearly. So I think the errmsg itself is
> enough.
>
>
>
Thanks, pushed.
I don't think it hurts to give some information on why this might happen,
so I did add an errdetail.
cheers
andrew
^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2026-04-30 14:54 UTC | newest]
Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-04-15 05:51 Fix a server crash problem from pg_get_database_ddl Chao Li <[email protected]>
2026-04-16 00:44 ` Jack Bonatakis <[email protected]>
2026-04-16 01:23 ` Japin Li <[email protected]>
2026-04-16 01:35 ` Chao Li <[email protected]>
2026-04-23 06:47 ` SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-26 14:50 ` Andrew Dunstan <[email protected]>
2026-04-27 02:02 ` Chao Li <[email protected]>
2026-04-27 04:56 ` SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-30 06:40 ` Chao Li <[email protected]>
2026-04-30 14:54 ` Andrew Dunstan <[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