Thank you for looking at this. On Fri, 7 Dec 2018 at 20:15, Michael Paquier <mich...@paquier.xyz> wrote: > -ATExecPartedIdxSetTableSpace(Relation rel, Oid newTableSpace) > +ATExecSetTableSpaceNoStorage(Relation rel, Oid newTableSpace) > NoStorage looks strange as routine name for this case. Would something > like ATExecPartedRelSetTableSpace be more adapted perhaps?
I'd say it's better to name the function in the more general purpose
way. Perhaps we'll get some relkind in the future that's not a
partitioned table or index that might need the reltablespace set. When
that happens, if we name the function the way you're proposing, then
we might end up with some other function which does the same thing, or
the patch adding the new relkind would have to rename the function to
something more like how I have it. To save from either of those
having to happen, would it not be better to give it the generic name
now?
Instead of renaming the function, I've altered the Assert() to allow
all relkinds which don't have storage and also updated the comment to
remove the mention of partitioned tables and indexes.
> + else if (stmt->partbound)
> + {
> + RangeVar *parent;
> + Relation parentrel;
> +
> + /*
> + * For partitions, when no other tablespace is specified, we default
> + * the tablespace to the parent partitioned table's.
> + */
> Okay, so the direct parent is what counts, and not the top-most parent.
> Could you add a test with multiple level of partitioned tables, like:
> - parent is in tablespace 1.
> - child is created in tablespace 2.
> - grandchild is created, which should inherit tablespace 2 from the
> child, but not tablespace 1 from the parent. In the existing example,
> as one partition is used to test the top-most parent and another for the
> new default, it looks cleaner to create a third partition which would be
> itself a partitioned table.
Sounds good. I've modified the existing test just to do it that way. I
don't think there's a need to have multiple tests for that.
I've attached an updated patch.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
v2-0001-Allow-newly-created-partitions-to-inherit-their-p.patch
Description: Binary data
