Date: Tue, 9 Apr 2019 16:35:07 -0400
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: David Rowley <david.rowley@2ndquadrant.com>
Cc: Andres Freund <andres@anarazel.de>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: pg_dump is broken for partition tablespaces
Message-ID: <20190409203507.GA25150@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CAKJS1f9fyFJQsAi-NJYvM+Bj3sSDooPL-tFcAm-nJtfzh9LprA@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

On 2019-Mar-07, David Rowley wrote:

> On Thu, 7 Mar 2019 at 11:37, Andres Freund <andres@anarazel.de> wrote:
> >
> > On 2019-03-07 11:31:15 +1300, David Rowley wrote:
> > > Do you think it's fine to reword the docs to make this point more
> > > clear, or do you see this as a fundamental problem with the patch?
> >
> > Hm, both? I mean I wouldn't necessarily characterize it as "fundamental"
> > problem, but ...
> 
> Okay, so if I understand you correctly, you're complaining about the
> fact that if the user does:
> 
> CREATE TABLE p (a int) PARTITION BY LIST(a) TABLESPACE pg_default;
> 
> that the user intended that all future partitions go to pg_default and
> not whatever default_tablespace is set to at the time?
> 
> If so, that seems like a genuine concern.

So, you don't need a partition in order to see a problem here: pg_dump
doesn't do the right thing for the partitioned table.  It does this:

SET default_tablespace = pg_default;
CREATE TABLE public.p (
    a integer
)
PARTITION BY LIST (a);

which naturally has a different effect on partitions.

Now, I think the problem for partitions can be solved in a simple
manner: we can have this code David quoted ignore the
MyDatabaseTableSpace exception for partitioned rels:

> /*
> * Never allow a pg_class entry to explicitly specify the database's
> * default tablespace in reltablespace; force it to zero instead. This
> * ensures that if the database is cloned with a different default
> * tablespace, the pg_class entry will still match where CREATE DATABASE
> * will put the physically copied relation.
> *
> * Yes, this is a bit of a hack.
> */
> if (reltablespace == MyDatabaseTableSpace)
> reltablespace = InvalidOid;

This gives the right effect AFAICS, namely to store the specified
tablespace regardless of what it is; and there is no problem with the
"physically copied relation" as the comment says, because there *isn't*
a physical relation in the first place.

However, in order to fix the pg_dump behavior for the partitioned rel,
we would need to emit the tablespace differently, i.e. not use SET
default_tablespace, but instead attach the tablespace clause to the
CREATE TABLE line.

I'll go have a look at what problems are there with doing that.

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