Re: pg_dump is broken for partition tablespaces

On 2019-Apr-17, Tom Lane wrote:

> Alvaro Herrera <alvherre@2ndquadrant.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