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