Date: Wed, 13 Jan 2021 09:57:00 -0300
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Michael Paquier <michael@paquier.xyz>
Cc: Tomas Vondra <tomas.vondra@2ndquadrant.com>,
	Tom Lane <tgl@sss.pgh.pa.us>, exclusion@gmail.com,
	pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #16577: Segfault on altering a table located in a dropped
 tablespace
Message-ID: <20210113125700.GA9451@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <X/6K9lAuRdfJHVLQ@paquier.xyz>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On 2021-Jan-13, Michael Paquier wrote:

> On Tue, Jan 12, 2021 at 12:08:04PM -0300, Alvaro Herrera wrote:
> > That would bloat the catalog with a lot of entries for stuff that can be
> > detected with the current method.  Did you notice that the code is
> > removing an "#ifdef NOT_USED" line to enable existing code?  Well, when
> > I wrote this code in 2005 (59d1b3d99e69) it was doing things as you
> > suggest, but in the end we decided that it wasn't necessary so it was
> > taken out.
> 
> I have not found a thread discussing that around the date of this
> commit, but I'll take your word on that.

I bet you didn't search pgsql-patches ;-)
https://www.postgresql.org/message-id/flat/20050703051522.GA13207%40surnet.cl

> I just have one small comment.
> 
> +   if (!create_storage && reltablespace != InvalidOid)
> +       recordDependencyOnTablespace(RelationRelationId, relid,
> +                                    reltablespace);
> For now we assume that this code path is taken only for partitioned
> tables or indexes per the logic in heap_create().  Perhaps it would be
> better to add to this code path, or to recordDependencyOnTablespace()
> an assertion to check that only the supported relkinds register this
> dependency?  If a new relkind is added, it would be easy to miss that
> this shared dependency may need to be supported.

Hmm ... the intent here is that if there is no storage, but a tablespace
is specified, then a dependency protects.  This should be agnostic to
relkind considerations.  I had first written the new symbol as
SHARED_DEPENDENCY_PARTITIONED_TABLE but then I realized the error of my
ways :-)

-- 
Álvaro Herrera       Valdivia, Chile
Tom: There seems to be something broken here.
Teodor: I'm in sackcloth and ashes...  Fixed.
        http://archives.postgresql.org/message-id/482D1632.8010507@sigaev.ru