Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1wISmj-0087aZ-1r for pgsql-hackers@arkaria.postgresql.org; Thu, 30 Apr 2026 14:54:38 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1wISmi-007zE5-2p for pgsql-hackers@arkaria.postgresql.org; Thu, 30 Apr 2026 14:54:36 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1wISmi-007zDw-1C for pgsql-hackers@lists.postgresql.org; Thu, 30 Apr 2026 14:54:36 +0000 Received: from mail-yx1-xb12c.google.com ([2607:f8b0:4864:20::b12c]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.98.2) (envelope-from ) id 1wISmf-000000041I7-2SY7 for pgsql-hackers@postgresql.org; Thu, 30 Apr 2026 14:54:35 +0000 Received: by mail-yx1-xb12c.google.com with SMTP id 956f58d0204a3-6530dd51ccaso849474d50.3 for ; Thu, 30 Apr 2026 07:54:33 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1777560872; cv=none; d=google.com; s=arc-20240605; b=MQzCWj0Jk/9Fta7tUdgGW0U9xuV6fzUY9wBkoQKyMecpfd+Y9mGsliwB62RsOReXBk 0/45U0dtMnTw+KdSv67sg+jqiH/pzySult1avFLg4EK9jrv4k0uV7/CjTjkuQh8v9ziT V/vt4u0ofRvTdCKnJzez3y12T6KLfLPMssjZjweQlftbncJhQLnGNFEc/MPBTsabwAQ7 TcwYVwfIGQAedNI6UYUyENhDL4nT+hoOKmPUwxSQAB92NRComIM0d+ohcUrp2HNbZv7/ deQloIos1Nqrpu6RZ10lPUwshbRufq2PPqvn4v8PPYBq9i/UXvQXVG9VbngUpyQLCZUj bx6w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:dkim-signature; bh=1DOg2gnD2v3WTLi27ZmVi5W+nQXJIrqJ3Vobj5oh4yw=; fh=rP455Jb8a1Z794ilKfjcd4n6Qnuj19RRimg8rNs13Cw=; b=LIE0+v85jxK+Hy7OvYoBeuCHmxHPR/5jmeRSlcfEjSYgXeGzwFBr6iBIYUclJKFZDp 6nfQXB9LQK75ujLAuTHVEEMOIxBDN3b56PsTUQadxjJU8g5Node4ZSWHVYLBmVdt3dsU s6ISixVeThSj0gyWDPQX2eP5hhIDAIjyNczMi4E5KIZpI8jVdxRrpAY0f0iC7GXGpplB ghI6yGUc+4wePOuTM0lnBKTOJIJiBAm4Ie+CSRbSanibMh3ran8x2Xfo550N34M4DK66 wF7uxhViJ8ExyWxk3n6yDbw2EnPjhCgA34wqdziIzrjC55Q+hg5UljeFhBcDI5gQxLRj 3BNA==; darn=postgresql.org ARC-Authentication-Results: i=1; mx.google.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dunslane-net.20251104.gappssmtp.com; s=20251104; t=1777560872; x=1778165672; darn=postgresql.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=1DOg2gnD2v3WTLi27ZmVi5W+nQXJIrqJ3Vobj5oh4yw=; b=lBAD3LeTzik3lsCRkBPf8mu8sqB5Jx4tS7OPQ+GDVTpUYzR0sDh06VEddeDDXaUy4n 07BOpKta0oA0JVarFGE7qrbcGL9plliYftxEj1Qxhgrj3VP9Gunsy7ZPGZ8MkLSmMbGi O0o6T6kVTVwqnAItVl3TGtL2ZA82dqWfxberUw579mpkpG86bx+4vVmnnQfe3qkz+KpX ve0sf3N7lxEoM+AE2gr6UKP2Ponugd6SKupzHyQCOnSyJxrHJ3V+dgE09j/Loq2OuhFC sz2cNDkP/lmKG3tim/DG72T+RUL2RWjF7KNUERmn419h/GL8UFAkKOpe4KykRd+Z2UF+ CH9g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1777560872; x=1778165672; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=1DOg2gnD2v3WTLi27ZmVi5W+nQXJIrqJ3Vobj5oh4yw=; b=nuPn4/3W7niQn6KgM7Xtaum5qp5hMTMfHWsKYO+YVwCMiQcRoF03v5IrqlzwZVu6dx bWYNbVrLYIfUTqU8N+p9VYWhC2YreSJhNew8YU6GYqJProCW0/sC3RO0kGgd4D/hHz8P sahLFH72D+5jzP1WTjxfOEwSAgeMtluIBVKNqWBbbMVxyQGgNlxBToQA9Q1pZaKECEFs 3grApdK2wVpYL24QuwLM/nOeJKFJxHjs8OcnZrSDVXWA0OfP68GcH+Wm0jLzvHUK+8i/ WUYUWvvaMDjjurEPWRQQqoridiu5LoP37SYb00kz4UQRpwckK4+9Rz5KQCQPor7jRY4D La9Q== X-Forwarded-Encrypted: i=1; AFNElJ+QTVivclECTyb6rgEp3vfnIAM5KpGINx/w1JHwT74ax8W23hCpsv8yYl3TKwzG3YjFyl7bD8GNLamms9nG@postgresql.org X-Gm-Message-State: AOJu0Yw5OZhHzTG8Gmiemx3L+XkFN0+zP/65ca2B0D5BRqkSMFH302e+ y1B/6QX3G3ZFOPXA/s5sDtNQSr6PRLS3j8suG4gLLHhdZh3pYPcUVXMGzwcWvZ+2r5Z8fVXw8ts 2n40YmGuZB61Nl3AyDKDTGyx5Z53mFEF7kbjeLgw7pQ== X-Gm-Gg: AeBDiesovdsJYapNv/6nlt2g6a5XIcI0DyIQYcTh5rlV16NwJ+c+BlcA2g3kzE4cup9 nIFPHbeFi+goJx2IaqR7Fxccbip+JkL0H2Q6zACQ7JR349onpPbAE2B69ktL7mSSdhV/W8sTk5k IQ1xjQ1GCGxij0o7IwH1YHzflVVLLOvSeoCx63RwIeeqI/xOn3h47yuP1iUbsS3s2ZVSaKdO51k OkRIEVMs+0G9vFI2ywtiCPoxaXqcEuVxoDSeLLlddYzsqUzTZDApB85Pi5NEaoXzN3Goq/UVD6g jDbeV3Zp0rrIfms9o9UPPvGaTzPziyc= X-Received: by 2002:a05:690e:4083:b0:650:19e5:879 with SMTP id 956f58d0204a3-65c18f35963mr2686885d50.44.1777560871443; Thu, 30 Apr 2026 07:54:31 -0700 (PDT) MIME-Version: 1.0 References: <573E45C1-31A4-4885-A00C-1A2171159A2A@gmail.com> <28b82ab2-5721-4e7c-bf71-979c3f198a2e@app.fastmail.com> <634F4A5D-9D38-4F9D-BC1C-70815CBB5089@gmail.com> <13310e0b-e2b0-45a2-873d-e2b51a8ea3b4@dunslane.net> <0777DBAC-9348-423A-AB51-5AAFDA445B95@gmail.com> In-Reply-To: From: Andrew Dunstan Date: Thu, 30 Apr 2026 10:54:20 -0400 X-Gm-Features: AVHnY4JOiP1Ly6So32_FJKpqNwuS-1PSMk-ZbOQSQPIxpPz5Sp6j4AclSB-bS1g Message-ID: Subject: Re: Fix a server crash problem from pg_get_database_ddl To: Chao Li Cc: SATYANARAYANA NARLAPURAM , Tom Lane , PostgreSQL-development , Japin Li Content-Type: multipart/alternative; boundary="0000000000004f2c460650aea484" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --0000000000004f2c460650aea484 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, Apr 30, 2026 at 2:41=E2=80=AFAM Chao Li wr= ote: > > > > On Apr 27, 2026, at 12:56, SATYANARAYANA NARLAPURAM < > satyanarlapuram@gmail.com> wrote: > > > > Hi Chao, > > > > On Sun, Apr 26, 2026 at 7:03=E2=80=AFPM Chao Li wrote: > > > > > > > On Apr 26, 2026, at 22:50, Andrew Dunstan 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/1538113.1768921841@sss.pgh.pa.us > > >> 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 thi= s. > > >> > > >> 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 erro= r > 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 =3D=3D 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 w= e > do not actually know the root cause. A concurrent DROP TABLESPACE could b= e > 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 --0000000000004f2c460650aea484 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Thu, Apr 30,= 2026 at 2:41=E2=80=AFAM Chao Li <li.evan.chao@gmail.com> wrote:


> On Apr 27, 2026, at 12:56, SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com<= /a>> wrote:
>
> Hi Chao,
>
> On Sun, Apr 26, 2026 at 7:03=E2=80=AFPM Chao Li <li.evan.chao@gmail.com> wr= ote:
>
>
> > On Apr 26, 2026, at 22:50, Andrew Dunstan <andrew@dunslane.net> 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/1538113.1768921841@sss.pgh.pa.us
> >> In this case, it is a very corner case but not something supe= ruser intentionally breaks.
> >> For example, a concurrent tablespace drop + database ddl to a= ssign a different tablespace or default.
> >> We aren't acquiring Access Share lock on the DB in this f= unction (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 fix= ing this.
> >>
> >> Attached an injection point test to show the race. Not intend= ed 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. Raisin= g 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 th= e hint might not be necessary.
>
> + if (spcname =3D=3D NULL)
> + ereport(ERROR,
> + errcode(ERRCODE_UNDEFINED_OBJECT),
> + errmsg("tablespace with OID %u does not exist",
> +=C2=A0 =C2=A0dbform->dattablespace));
> +
>
> A message with error detail that says a concurrent DDL could have drop= ped 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 resu= lt, 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 e= nough.



Thanks, pushed.

I don't think it hurts to give some information on why this migh= t happen, so I did add an errdetail.

cheers
<= div>
andrew
=C2=A0
--0000000000004f2c460650aea484--