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

On Mon, 10 Dec 2018 at 15:22, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> - Assert(rel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX);
> + Assert(rel->rd_rel->relkind == RELKIND_VIEW ||
> + rel->rd_rel->relkind == RELKIND_COMPOSITE_TYPE ||
> + rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE ||
> + rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE ||
> + rel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX);
>
> Alvaro has begun a thread about that recently, so it seems to me that
> this assertion could become invalid (per se my comments down-thread):
> https://www.postgresql.org/message-id/20181206215552.fm2ypuxq6nhpwjuc@alvherre.pgsql
>
> Of course, it depends on the conclusion of this other thread. It will
> perhaps make sense at some point to review on which relkinds this
> function can work on, but I would recommend to review that behavior when
> it actually makes sense and keep the current interface simple. So your
> first version looked fine to me on those grounds. Another important
> thing which would help with a restricted assertion is that if this code
> path begins to be taken for other relkinds then the developer who
> implements a new feature depending on it will need to look at this code
> and consider the use-case behind it, instead of assuming that it will
> hopefully just work.

Likely it would be nice if we had a macro to determine if the relkind
should have storage or not, and then just Assert() we get one of those
into the function.

For now, I've just changed the Assert to only pass when we get a
partitioned table or partitioned index. I've left a comment to mention
that other types may need to be added later. Assert failures should
remind us to check that any newly added callers work correctly.

Updated patch attached.

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