Re: pg_dump emits ALTER TABLE ONLY partitioned_table

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Noah Misch <noah(at)leadboat(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pg_dump emits ALTER TABLE ONLY partitioned_table
Date: 2017-04-25 16:26:50
Message-ID: 20170425162650.GN9812@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Robert,

* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> On Mon, Apr 24, 2017 at 9:17 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > I wonder why the restriction is there, which is probably part of the
> > reason that I'm thinking of phrasing the documentation that way.
> >
> > Beyond a matter of round to-its, is there a reason why it couldn't (or
> > shouldn't) be supported? I'm not suggesting now, of course, but in a
> > future release.
>
> I don't see any particular reason, but I haven't looked into the
> matter deeply. One thing to consider is that you can already more or
> less do exactly that thing, like this:
>
> rhaas=# create table foo (a int, b text) partition by range (a);
> CREATE TABLE
> Time: 4.738 ms
> rhaas=# create table foo1 partition of foo for values from (0) to (100);
> CREATE TABLE
> Time: 5.162 ms
> rhaas=# insert into foo1 select 1, 'snarf' from generate_series(1,10000000) g;
> INSERT 0 10000000
> Time: 12835.153 ms (00:12.835)
> rhaas=# alter table foo1 add constraint xyz check (b != 'nope');
> ALTER TABLE
> Time: 1059.728 ms (00:01.060)
> rhaas=# alter table foo add constraint xyz check (b != 'nope');
> NOTICE: merging constraint "xyz" with inherited definition
> ALTER TABLE
> Time: 1.243 ms
>
> So we may not really need another way to do it.

Interesting. Seems like the question is really what we mean by "ONLY"
here. For my 2c, at least, if we can check that all of the partitions
already have the constraint enforced, such that the only thing we're
changing is the partitioned table, then that's exactly what "ONLY"
should do. That would require a bit of rework, presumably, of how that
keyword is handled but the basics are all there.

> >> Also, I think saying that it it will result in an error is a bit more
> >> helpful than saying that it is is not supported, since the latter
> >> might lead someone to believe that it could result in undefined
> >> behavior (i.e. arbitrarily awful misbehavior) which is not the case.
> >> So I like what he wrote, for whatever that's worth.
> >
> > I don't mind stating that it'll result in an error, I just don't want to
> > imply that there's some way to get around that error if things were done
> > differently.
> >
> > How about:
> >
> > ---
> > Once partitions exist, using <literal>ONLY</literal> will always result
> > in an error as adding or dropping constraints on only the partitiioned
> > table, when partitions exist, is not supported.
> > ---
>
> I still prefer Amit's language, but it's not worth to me to keep
> arguing about it. If you prefer what you've written there, fine, but
> let's get something committed and move on. I think we understand what
> needs to be fixed here now, and I'd like to do get that done. If you
> don't want to do it or don't have time to do it, I'll pick it up
> again, but let's not let this keep dragging out.

I'm planning to push just this patch to make the backend accept the
ALTER TABLE ONLY when no partitions exist later this afternoon, but the
work here isn't done as noted in my other email.

Thanks!

Stephen

Attachment Content-Type Size
fix_alter_table_only_partitions_v3-master.patch text/x-diff 14.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Claudio Freire 2017-04-25 16:37:13 Re: PG 10 release notes
Previous Message Fabien COELHO 2017-04-25 16:22:47 Re: pgbench tap tests & minor fixes