Re: pg_dump is broken for partition tablespaces

On 2019-Mar-06, David Rowley wrote:

> Over on [1] Andres pointed out that the pg_dump support for the new to
> PG12 tablespace inheritance feature is broken. This is the feature
> added in ca4103025dfe26 to allow a partitioned table to have a
> tablespace that acts as the default tablespace for newly attached
> partitions. The idea being that you can periodically change the
> default tablespace for new partitions to a tablespace that sits on a
> disk partition with more free space without affecting the default
> tablespace for normal non-partitioned tables. Anyway...
>
> pg_dump is broken with this.

Here's a patch to fix the reported problems. It's somewhat invasive,
and I've spent a long time staring at it, so I very much appreciate eyes
on it.

Guiding principles:

* Partitioned tables can have the database tablespace in their
reltablespace column, in contrast with non-partitioned relations.
This is okay and doesn't cause permission checks, since nothing
is put in the tablespace just yet.

* When creating a partition, the parent's tablespace takes precedence
over default_tablespace. This sounds a bit weird at first, but I
think it's correct. If you really want your partition to override the
parent's tablespace specification, you need to add a tablespace
clause. (I was initially opposed to this, but on further reflection
I think it's the right thing to do.)

With these things in mind, I have introduced some behavior changes. In
particular, both bugs mentioned in this thread have been fixed.

* pg_dump now correctly reproduces the state, still without using any
TABLESPACE clauses. (I suppose this is important for portability of
dumps, as well as the --no-tablespaces option).

* When a partitioned table is created specifying the database tablespace
in the TABLESPACE clause, and later a partition is created under
a different non-empty default_tablespace setting, the partition honors
the parent's tablespace. (Fixing this bug became one of the
principles here.)

* When an index is rewritten because of an ALTER TABLE, it no longer
moves magically to another tablespace. This worked fine for
non-partitioned indexes (commit bd673e8e864a fixed it), but it was
broken for partitioned ones.

* pg_dump was patched to emit straight CREATE TABLE plus ALTER TABLE
ATTACH for partitions (just like the binary upgrade code does) instead
of CREATE TABLE PARTITION OF. This is what lets the partition use a
straight default_tablespace setting instead of having to conditionally
attach TABLESPACE clauses, which would create quite a mess.

Making this work required somewhat unusual hacks:

* Nodes IndexStmt and Constraint have gained a new member
"reset_default_tblspc". This is not set in the normal code paths, but
when ALTER TABLE wants to recreate an index, it sets this flag; the
flag tells index creation to reset default_tablespace to empty. This
is only necessary because ALTER TABLE execution resolves (at execution
time) the tablespace of the artifically-generated SQL command to
recreate the index. If default_tablespace is left there, it
interferes with that.

* GetDefaultTablespace now behaves differently for partitioned tables,
precisely because we want it not to return InvalidOid when the
tablespace is specifically selected.

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