Re: pg_dump is broken for partition tablespaces

On 2019-Apr-17, Tom Lane wrote:

> Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> writes:
> > 1. pg_dump now uses regular CREATE TABLE followed by ALTER TABLE / ATTACH
> > PARTITION when creating partitions, rather than CREATE TABLE
> > PARTITION OF. pg_dump --binary-upgrade was already doing that, so
> > this part mostly removes some code. In order to make the partitions
> > reach the correct tablespace, the "default_tablespace" GUC is used.
> > No TABLESPACE clause is added to the dump. This is David's patch
> > near the start of the thread.
>
> This idea seems reasonable independently of all else, simply on the grounds
> of reducing code duplication. It also has the advantage that if you try
> to do a selective restore of just a partition, and the parent partitioned
> table isn't around, you can still do it (with an ignorable error).

I'll get this part pushed, then.

> > 2. When creating a partition using the CREATE TABLE PARTITION OF syntax,
> > the TABLESPACE clause has highest precedence; if that is not given,
> > the partitioned table's tablespace is used; if that is set to 0 (the
> > default), default_tablespace is used; if that's set to empty or a
> > nonexistant tablespace, the database's default tablespace is used.
> > This is (I think) what Andres proposed in
> > https://postgr.es/m/20190306223741.lolaaimhkkp4kict@alap3.anarazel.de
>
> Hmm. The precedence order between the second and third options seems
> pretty arbitrary and hence unrememberable. I don't say this choice is
> wrong, but it's not clear that it's right, either.

Well, I see it as the default_tablespace being a global setting whereas
the parent is "closer" to the partition definition, which is why it has
higher priority. I don't have a strong opinion however (and I think the
patch would be shorter if default_tablespace had higher precedence.)

Maybe others care to comment?

> > 3. Partitioned relations can have the database tablespace in
> > pg_class.reltablespace, as opposed to storage-bearing relations which
> > cannot. This is useful to be able to put partitions in the database
> > tablespace even if the default_tablespace is set to something else.
>
> I still feel that this is a darn bad idea. It goes against the rule
> that's existed for pg_class.reltablespace since its beginning, and
> I think it's inevitable that that's going to break something.

Yes, this deviates from current practice, and while I tested this in as
many ways as I could think of, I cannot deny that it might break
something unexpectedly.

Here's a v4 btw, which is just some adjustments to the regress test
script and expected file.

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services