Re: Should new partitions inherit their tablespace from their parent?

On Mon, 17 Dec 2018 at 11:07, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> First, I changed the Assert() to use the macro for relations with
> storage that I just posted in the other thread that Michael mentioned.
>
> I then noticed that we're doing a heap_openrv() in the parent relation
> and closing it before MergeAttribute() does the same thing all over
> again; also MergeAttribute has the silly array-of-OIDs return value for
> the parents so that DefineRelation can handle it again later.  Rube
> Goldberg says hi.  I changed this so that *before* doing anything with
> the parent list, we transform it to a list of OIDs, and lock them; so
> MergeAttributes now receives the list of OIDs of parents rather than
> RangeVars.  We can also move the important comment (about lock level of
> parent rels) buried in the bowels of MergeAttribute to the place where
> it belongs in DefineRelation; and we no longer have to mess with
> transforming names to OIDs multiple times.
>
> Proposed patch attached.

I like this idea. Seems much better than resolving the relation name twice.

I've read over the patch and the only two things that I noted were:

1. Shouldn't you be using the RangeVarGetRelid() macro instead of
calling RangeVarGetRelidExtended()?
2. In MergeAttributes(), the parentOids list still exists and is
populated.  This is now only used to determine if the "supers" list
contains any duplicate Oids. Maybe it's better to rename that list to
something like "seenOids" to avoid any confusion with the "supers"
list. Or maybe it's worth thinking of a better way to detect duplicate
items in the "supers" list.

-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services