Date: Thu, 11 Sep 2025 15:42:16 -0700
From: Noah Misch <noah@leadboat.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Smolkin Grigory <smallkeen@gmail.com>, pgsql-hackers@postgresql.org
Subject: Re: race condition in pg_class
Message-ID: <20250911224216.4f.nmisch@google.com>
References: <20231027184832.9b.nmisch@google.com>
 <1596629.1698435146@sss.pgh.pa.us>
 <20231027214946.79.nmisch@google.com>
 <1617596.1698446455@sss.pgh.pa.us>
 <20231027232612.99.nmisch@google.com>
 <20231102030915.d3.nmisch@google.com>
 <20240512232923.aa.nmisch@google.com>
 <1802710.1757608564@sss.pgh.pa.us>
 <20250911185729.ad.nmisch@google.com>
 <1826823.1757619287@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1826823.1757619287@sss.pgh.pa.us>
User-Agent: Mutt/2.2.12 (2023-09-09)
Archived-At: 
 <https://www.postgresql.org/message-id/20250911224216.4f.nmisch%40google.com>
Precedence: bulk

On Thu, Sep 11, 2025 at 03:34:47PM -0400, Tom Lane wrote:
> Noah Misch <noah@leadboat.com> writes:
> > On Thu, Sep 11, 2025 at 12:36:04PM -0400, Tom Lane wrote:
> >> It emerges that the "ALTER DATABASE regression_utf8 RESET TABLESPACE"
> >> command is a complete no-op [1].  I am guessing that that was not the
> >> behavior you had in mind, and am wondering if we are losing any test
> >> coverage thereby.  Did you have a specific reason for manipulating the
> >> TABLESPACE property and not some random GUC setting?
> 
> > I have no specific notes or memories about that RESET TABLESPACE, but I likely
> > wanted "SET TABLESPACE pg_default".  RESET TABLESPACE may still have the
> > effect of loading a catcache entry, as a later comment says:
> > ...
> > I'm inclined to change it as
> > attached.  This doesn't reduce database.sql test coverage of dbcommands.c or
> > catcache.c, so I'll bet it doesn't lose anything vs. the original database.sql
> > commit.  Some check-tests runs showed it adding database.sql coverage of
> > catcache.c:1908-1911 and catcache.c:1983-1984, so that's a bonus.
> 
> Thanks, that looks sane.
> 
> > Do you plan to back-patch that?  That should dictate whether I back-patch the
> > test change.
> 
> That change will convert some commands that are currently no-ops into
> errors, which doesn't seem like a great thing to do in minor releases.

My default would be no back-patches of the above, but one could defend either
choice.  The "error if some pg_db_role_setting entry exists, else no error"
behavior (postgr.es/m/1791672.1757607520@sss.pgh.pa.us) means the no-op
outcome already isn't 100%.  That reduces the risk that someone is relying on
the no-op, but some risk remains.

> However ... it might still be okay to cram it into v18.  Do you have
> an opinion about that?

Feels like something I wouldn't do, but I don't have a concrete reason.