Re: pg_dump is broken for partition tablespaces

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, 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-18 21:50:31
Message-ID: 20190418215031.GA11418@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2019-Apr-17, Alvaro Herrera wrote:

> On 2019-Apr-17, Tom Lane wrote:
>
> > Alvaro Herrera <alvherre(at)2ndquadrant(dot)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.

After looking at it again, I found that there's no significant
duplication reduction -- the patch simply duplicates one block in a
different location, putting half of the original code in each. And if we
reject the idea of separating tablespaces, there's no reason to do
things that way. So ISTM if we don't want the tablespace thing, we
should not apply this part. FWIW, we got quite a few positive votes for
handling tablespaces this way for partitioned tables [1] [2], so I
resist the idea that we have to revert the initial commit, as some seem
to be proposing.

After re-reading the thread one more time, I found one more pretty
reasonable point that Andres was complaining about, and I made things
work the way he described. Namely, if you do this:

SET default_tablespace TO 'foo';
CREATE TABLE part (a int) PARTITION BY LIST (a);
SET default_tablespace TO 'bar';
CREATE TABLE part1 PARTITION OF part FOR VALUES IN (1);

then the partition must end up in tablespace bar, not in tablespace foo:
the reason is that the default_tablespace is not "strong enough" to
stick with the partitioned table. The partition would only end up in
tablespace foo in this case:

CREATE TABLE part (a int) PARTITION BY LIST (a) TABLESPACE foo;
CREATE TABLE part1 PARTITION OF part FOR VALUES IN (1);

i.e. when the tablespace is explicitly indicated in the CREATE TABLE
command for the partitioned table. Of course, you can still add a
TABLESPACE clause to the partition to override it (and you can change
the parent table's tablespace later.)

So here's a proposed v5.

I would appreciate others' eyes on this patch.

[1] https://postgr.es/m/CAKJS1f9SxVzqDrGD1teosFd6jBMM0UEaa14_8mRvcWE19Tu0hA@mail.gmail.com
[2] https://www.postgresql.org/message-id/flat/CAKJS1f9PXYcT%2Bj%3DoyL-Lquz%3DScNwpRtmD7u9svLASUygBdbN8w%40mail.gmail.com

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

Attachment Content-Type Size
v5-0001-Make-pg_dump-emit-ATTACH-PARTITION-instead-of-PAR.patch text/x-diff 9.8 KB
v5-0002-fix-behavior-to-consider-default_tablespace.patch text/x-diff 47.1 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2019-04-18 21:52:38 Re: finding changed blocks using WAL scanning
Previous Message Bruce Momjian 2019-04-18 21:47:56 Re: finding changed blocks using WAL scanning