Re: pg_dump is broken for partition tablespaces

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: pg_dump is broken for partition tablespaces
Date: 2019-04-09 20:35:07
Message-ID: 20190409203507.GA25150@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2019-Mar-07, David Rowley wrote:

> On Thu, 7 Mar 2019 at 11:37, Andres Freund <andres(at)anarazel(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2019-04-09 20:46:24 Re: PageGetFreeSpace() isn't quite the right thing for some of its callers
Previous Message Ashwin Agrawal 2019-04-09 20:24:37 Re: Zedstore - compressed in-core columnar storage