> 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/






Reply via email to