Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

Alvaro Herrera <alvherre@alvh.no-ip.org>

From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: Amit Langote <amitlangote09@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2020-12-01T15:25:19Z
Lists: pgsql-hackers
Hi Justin,

Thanks for all the comments. I'll incorporate everything and submit an
updated version later.

On 2020-Nov-30, Justin Pryzby wrote:

> On Tue, Nov 03, 2020 at 08:56:06PM -0300, Alvaro Herrera wrote:

> > +++ b/src/bin/psql/describe.c
> > -			printfPQExpBuffer(&tmpbuf, _("Partition of: %s %s"), parent_name,
> > -							  partdef);
> > +			printfPQExpBuffer(&tmpbuf, _("Partition of: %s %s%s"), parent_name,
> > +							  partdef,
> > +							  strcmp(detached, "t") == 0 ? " DETACHED" : "");
> 
> The attname "detached" is a stretch of what's intuitive (it's more like
> "detachING" or half-detached).  But I think psql should for sure show something
> more obvious to users.  Esp. seeing as psql output isn't documented.  Let's
> figure out what to show to users and then maybe rename the column that, too.

OK.  I agree that "being detached" is the state we want users to see, or
maybe "detach pending", or "unfinisheddetach" (ugh).  I'm not sure that
pg_inherits.inhbeingdetached" is a great column name.  Opinions welcome.

> > +PG_KEYWORD("finalize", FINALIZE, UNRESERVED_KEYWORD, BARE_LABEL)
> 
> Instead of finalize .. deferred ?  Or ??

Well, I'm thinking that this has to be a verb in the imperative mood.
The user is commanding the server to "finalize this detach operation".
I'm not sure that DEFERRED fits that grammatical role.  If there are
other ideas, let's discuss them.

ALTER TABLE tst DETACH PARTITION tst_1 FINALIZE  <-- decent
ALTER TABLE tst DETACH PARTITION tst_1 COMPLETE  <-- I don't like it
ALTER TABLE tst DETACH PARTITION tst_1 DEFERRED  <-- grammatically faulty?

> ATExecDetachPartition:
> Doesn't this need to lock the table before testing for default partition ?

Correct, it does.

> I ended up with apparently broken constraint when running multiple loops around
> a concurrent detach / attach:
> 
> while psql -h /tmp postgres -c "ALTER TABLE p ATTACH PARTITION p1 FOR VALUES FROM (1)TO(2)" -c "ALTER TABLE p DETACH PARTITION p1 CONCURRENTLY"; do :; done&
> while psql -h /tmp postgres -c "ALTER TABLE p ATTACH PARTITION p1 FOR VALUES FROM (1)TO(2)" -c "ALTER TABLE p DETACH PARTITION p1 CONCURRENTLY"; do :; done&
> 
>     "p1_check" CHECK (true)
>     "p1_i_check" CHECK (i IS NOT NULL AND i >= 1 AND i < 2)

Not good.



Commits

  1. Make detach-partition-concurrently-4 less timing sensitive

  2. Improve documentation on DETACH PARTITION lock levels

  3. Track detached partitions more accurately in partdescs

  4. Allow a partdesc-omitting-partitions to be cached

  5. Fix relcache inconsistency hazard in partition detach

  6. Don't add a redundant constraint when detaching a partition

  7. ALTER TABLE ... DETACH PARTITION ... CONCURRENTLY

  8. Let ALTER TABLE Phase 2 routines manage the relation pointer

  9. Check default partitions constraints while descending