Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-05-25 Thread Alvaro Herrera
On 2021-May-24, Noah Misch wrote: > prairiedog and wrasse failed a $SUBJECT test after this (commit 8aba932). > Also, some non-CLOBBER_CACHE_ALWAYS animals failed a test before the fix. > These IsolationCheck failures match detach-partition-concurrently[^\n]*FAILED: > > sysname │ snapsho

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-05-24 Thread Amit Langote
On Mon, May 24, 2021 at 6:07 PM Noah Misch wrote: > On Wed, Apr 21, 2021 at 04:38:55PM -0400, Alvaro Herrera wrote: > > [fix to let CLOBBER_CACHE_ALWAYS pass] > > > Barring objections, I will get this pushed early tomorrow. > > prairiedog and wrasse failed a $SUBJECT test after this (commit 8aba93

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-05-24 Thread Noah Misch
On Wed, Apr 21, 2021 at 04:38:55PM -0400, Alvaro Herrera wrote: [fix to let CLOBBER_CACHE_ALWAYS pass] > Barring objections, I will get this pushed early tomorrow. prairiedog and wrasse failed a $SUBJECT test after this (commit 8aba932). Also, some non-CLOBBER_CACHE_ALWAYS animals failed a test

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-05-12 Thread Amit Langote
On Fri, May 7, 2021 at 2:13 AM Álvaro Herrera wrote: > On 2021-Apr-30, Amit Langote wrote: > > > The case I was looking at is when a partition detach appears as > > in-progress to a serializable transaction. > > Yeah, I was exceedingly sloppy on my reasoning about this, and you're > right that tha

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-05-06 Thread Álvaro Herrera
On 2021-May-06, Amit Langote wrote: > That makes sense, thanks for noticing. > > How about the attached? I tweaked the linkage; as submitted, the text in the link contained what is in the tag, so literally it had: ... see DETACH PARTITION partition_name [ CONCURRENTLY | FINALIZE ] for deta

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-05-06 Thread Álvaro Herrera
On 2021-May-05, Pavel Luzanov wrote: > Hello, > > I found this in the documentation, section '5.11.3. Partitioning Using > Inheritance'[1]: > "Some operations require a stronger lock when using declarative partitioning > than when using table inheritance. For example, removing a partition from a

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-05-06 Thread Álvaro Herrera
On 2021-Apr-30, Amit Langote wrote: > The case I was looking at is when a partition detach appears as > in-progress to a serializable transaction. Yeah, I was exceedingly sloppy on my reasoning about this, and you're right that that's what actually happens rather than what I said. > If the calle

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-05-06 Thread Pavel Luzanov
On 06.05.2021 08:35, Amit Langote wrote: On Wed, May 5, 2021 at 7:59 PM Pavel Luzanov wrote: I found this in the documentation, section '5.11.3. Partitioning Using Inheritance'[1]: "Some operations require a stronger lock when using declarative partitioning than when using table inheritance. F

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-05-05 Thread Amit Langote
On Wed, May 5, 2021 at 7:59 PM Pavel Luzanov wrote: > I found this in the documentation, section '5.11.3. Partitioning Using > Inheritance'[1]: > "Some operations require a stronger lock when using declarative partitioning > than when using table inheritance. For example, removing a partition fr

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-05-05 Thread Pavel Luzanov
Hello, I found this in the documentation, section '5.11.3. Partitioning Using Inheritance'[1]: "Some operations require a stronger lock when using declarative partitioning than when using table inheritance. For example, removing a partition from a partitioned table requires taking an ACCESS EX

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-30 Thread Amit Langote
(Thanks for committing the fix.) On Thu, Apr 29, 2021 at 1:11 AM Álvaro Herrera wrote: > On Wed, Apr 28, 2021, at 10:21 AM, Amit Langote wrote: > I noticed that rd_partdesc_nodetached_xmin can sometimes end up with > value 0. While you seem to be already aware of that, because otherwise > you wou

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-28 Thread Álvaro Herrera
Pushed that now, with a one-line addition to the docs that only one partition can be marked detached. -- Álvaro Herrera39°49'30"S 73°17'W "That sort of implies that there are Emacs keystrokes which aren't obscure. I've been using it daily for 2 years now and have yet t

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-28 Thread Álvaro Herrera
Thanks for re-reviewing! This one I hope is the last version. On Wed, Apr 28, 2021, at 10:21 AM, Amit Langote wrote: > I noticed that rd_partdesc_nodetached_xmin can sometimes end up with > value 0. While you seem to be already aware of that, because otherwise > you wouldn't have added Transaction

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-28 Thread Amit Langote
On Wed, Apr 28, 2021 at 8:32 AM Alvaro Herrera wrote: > On 2021-Apr-27, Alvaro Herrera wrote: > > > This v3 handles things as you suggested and works correctly AFAICT. I'm > > going to add some more tests cases to verify the behavior in the > > scenarios you showed, and get them to run under cach

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-27 Thread Alvaro Herrera
On 2021-Apr-27, Alvaro Herrera wrote: > This v3 handles things as you suggested and works correctly AFAICT. I'm > going to add some more tests cases to verify the behavior in the > scenarios you showed, and get them to run under cache-clobber options to > make sure it's good. Yep, it seems to wo

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-27 Thread Alvaro Herrera
This v3 handles things as you suggested and works correctly AFAICT. I'm going to add some more tests cases to verify the behavior in the scenarios you showed, and get them to run under cache-clobber options to make sure it's good. Thanks! -- Álvaro Herrera Valdivia, Chile >From 145ed63c43

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-27 Thread Alvaro Herrera
On 2021-Apr-27, Amit Langote wrote: > On Tue, Apr 27, 2021 at 4:34 PM Amit Langote wrote: > I think we may need a separate context for partdesc_nodetached, likely > with the same kludges as rd_pdcxt. Maybe the first problem will go > away with that as well. Ooh, seems I completely misunderstoo

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-27 Thread Amit Langote
On Tue, Apr 27, 2021 at 4:34 PM Amit Langote wrote: > Thanks for the updated patch. I've been reading it, but I noticed a > bug in 8aba9322511f, which I thought you'd want to know to make a note > of when committing this one. > > So we decided in 8aba9322511f that it is okay to make the memory >

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-27 Thread Amit Langote
Thanks for the updated patch. I've been reading it, but I noticed a bug in 8aba9322511f, which I thought you'd want to know to make a note of when committing this one. So we decided in 8aba9322511f that it is okay to make the memory context in which a transient partdesc is allocated a child of Po

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-26 Thread Alvaro Herrera
Sorry, I forgot to update some comments in that version. Fixed here. -- Álvaro Herrera39°49'30"S 73°17'W >From cb6d9e026624656e826ea880716ee552b15203a8 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Mon, 26 Apr 2021 14:53:04 -0400 Subject: [PATCH v2] Allow a par

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-26 Thread Alvaro Herrera
On 2021-Apr-26, Alvaro Herrera wrote: > > Please allow me to study the patch a bit more closely and get back tomorrow. > > Sure, thanks! Here's a more polished version. After trying the version with the elog(ERROR) when two detached partitions are present, I decided against it; it is unhelpful

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-26 Thread Alvaro Herrera
Hello Amit, On 2021-Apr-26, Amit Langote wrote: > On Sat, Apr 24, 2021 at 8:31 AM Alvaro Herrera > wrote: > > I haven't added a mechanism to verify this; but with asserts on, this > > patch will crash if you have more than one. I think the behavior is not > > necessarily sane with asserts off

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-26 Thread Amit Langote
Hi Alvaro, On Sat, Apr 24, 2021 at 8:31 AM Alvaro Herrera wrote: > On 2021-Apr-23, Alvaro Herrera wrote: > > I think the patch I posted was too simple. I think a real fix requires > > us to keep track of exactly in what way the partdesc is outdated, so > > that we can compare to the current situ

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-23 Thread Alvaro Herrera
On 2021-Apr-23, Alvaro Herrera wrote: > I think the patch I posted was too simple. I think a real fix requires > us to keep track of exactly in what way the partdesc is outdated, so > that we can compare to the current situation in deciding to use that > partdesc or build a new one. For example,

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-23 Thread Alvaro Herrera
On 2021-Apr-23, Amit Langote wrote: > Back in the 1st session: > > end; > insert into fk select generate_series(1, 1); > INSERT 0 1 > Time: 47400.792 ms (00:47.401) I guess I was wrong about that ... the example I tried didn't have 1000s of partitions, and I used debug print-outs to show

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-23 Thread Amit Langote
On Fri, Apr 23, 2021 at 4:26 AM Alvaro Herrera wrote: > On 2021-Apr-22, Amit Langote wrote: > > -* The reason for this check is that we want to avoid seeing the > > +* The reason for this hack is that we want to avoid seeing the > > * partition as alive in RI queries durin

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-22 Thread Alvaro Herrera
On 2021-Apr-22, Amit Langote wrote: > On Thu, Apr 22, 2021 at 5:39 AM Alvaro Herrera > wrote: > Reading through the latest one, seeing both include_detached and > omit_detached being used in different parts of the code makes it a bit > hard to keep in mind what a given code path is doing wrt de

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-22 Thread Amit Langote
(Sorry about being away from this for over a week.) On Thu, Apr 22, 2021 at 5:39 AM Alvaro Herrera wrote: > While the approach in the previous email does pass the tests, I think > (but couldn't find a test case to prove) it does so coincidentally, not > because it is correct. If I make the test

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-21 Thread Alvaro Herrera
On 2021-Apr-10, Justin Pryzby wrote: > If it *implies* the partition constraint, then it's at least as tight (and > maybe tighter), yes ? > > I think you're concerned with the case that someone has a partition with > "tight" bounds like (a>=200 and a<300) and a check constraint that's "less > tig

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-21 Thread Alvaro Herrera
While the approach in the previous email does pass the tests, I think (but couldn't find a test case to prove) it does so coincidentally, not because it is correct. If I make the test for "detached exist" use the cached omits-partitions-partdesc, it does fail, because we had previously cached one

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-20 Thread Alvaro Herrera
Actually I had a silly bug in the version that attempted to cache a partdesc that omits detached partitions. This one, while not fully baked, seems to work correctly (on top of the previous one). The thing that I don't fully understand is why we have to require to have built the regular one first

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-20 Thread Alvaro Herrera
OK so after mulling this over for a long time, here's a patch for this. It solves the problem by making the partition descriptor no longer be cached if a detached partition is omitted. Any transaction that needs a partition descriptor that excludes detached partitions, will have to recreate the pa

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-13 Thread Justin Pryzby
On Sat, Apr 10, 2021 at 01:42:26PM -0500, Justin Pryzby wrote: > On Sun, Mar 21, 2021 at 03:01:15PM -0300, Alvaro Herrera wrote: > > > But note that it doesn't check if an existing constraint "implies" the new > > > constraint - maybe it should. > > > > Hm, I'm not sure I want to do that, because

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-13 Thread Alvaro Herrera
On 2021-Apr-13, Amit Langote wrote: > Actually it occurred to me this morning that CLOBBER_CACHE_ALWAYS is > what exposed this problem on this animal (not sure if other such > animals did too though). With CLOBBER_CACHE_ALWAYS, a PartitionDesc > will be built afresh on most uses. In this particu

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-12 Thread Amit Langote
On Mon, Apr 12, 2021 at 6:23 AM Justin Pryzby wrote: > On Sun, Apr 11, 2021 at 05:20:35PM -0400, Alvaro Herrera wrote: > > On 2021-Mar-31, Tom Lane wrote: > > > > > diff -U3 > > > /home/buildfarm/trilobite/buildroot/HEAD/pgsql.build/src/test/isolation/expected/detach-partition-concurrently-4.out

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-12 Thread Amit Langote
On Mon, Apr 12, 2021 at 4:42 PM Amit Langote wrote: > On Mon, Apr 12, 2021 at 6:20 AM Alvaro Herrera > wrote: > > On 2021-Mar-31, Tom Lane wrote: > > > > > diff -U3 > > > /home/buildfarm/trilobite/buildroot/HEAD/pgsql.build/src/test/isolation/expected/detach-partition-concurrently-4.out > > >

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-12 Thread Amit Langote
On Mon, Apr 12, 2021 at 6:20 AM Alvaro Herrera wrote: > On 2021-Mar-31, Tom Lane wrote: > > > diff -U3 > > /home/buildfarm/trilobite/buildroot/HEAD/pgsql.build/src/test/isolation/expected/detach-partition-concurrently-4.out > > > > /home/buildfarm/trilobite/buildroot/HEAD/pgsql.build/src/test/i

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-11 Thread Justin Pryzby
On Sun, Apr 11, 2021 at 05:20:35PM -0400, Alvaro Herrera wrote: > On 2021-Mar-31, Tom Lane wrote: > > > diff -U3 > > /home/buildfarm/trilobite/buildroot/HEAD/pgsql.build/src/test/isolation/expected/detach-partition-concurrently-4.out > > > > /home/buildfarm/trilobite/buildroot/HEAD/pgsql.build/

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-11 Thread Alvaro Herrera
On 2021-Mar-31, Tom Lane wrote: > diff -U3 > /home/buildfarm/trilobite/buildroot/HEAD/pgsql.build/src/test/isolation/expected/detach-partition-concurrently-4.out > > /home/buildfarm/trilobite/buildroot/HEAD/pgsql.build/src/test/isolation/output_iso/results/detach-partition-concurrently-4.out >

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-04-10 Thread Justin Pryzby
On Sun, Mar 21, 2021 at 03:01:15PM -0300, Alvaro Herrera wrote: > > But note that it doesn't check if an existing constraint "implies" the new > > constraint - maybe it should. > > Hm, I'm not sure I want to do that, because that means that if I later > have to attach the partition again with the

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-03-31 Thread Tom Lane
Alvaro Herrera writes: > I added that test as promised, and I couldn't find any problems, so I > have pushed it. Buildfarm testing suggests there's an issue under CLOBBER_CACHE_ALWAYS: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=trilobite&dt=2021-03-29%2018%3A14%3A24 specifically d

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-03-25 Thread Alvaro Herrera
On 2020-Nov-30, Justin Pryzby wrote: > On Tue, Nov 03, 2020 at 08:56:06PM -0300, Alvaro Herrera wrote: > > * On the first transaction (the one that marks the partition as > > detached), the partition is locked with ShareLock rather than > > ShareUpdateExclusiveLock. This means that DML is not al

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-03-25 Thread Alvaro Herrera
I added that test as promised, and I couldn't find any problems, so I have pushed it. Thanks! -- Álvaro Herrera Valdivia, Chile

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-03-25 Thread Alvaro Herrera
On 2021-Mar-23, Alvaro Herrera wrote: > I think a possible solution to this problem is that the "detach" flag in > pg_inherits is not a boolean anymore, but an Xid (or maybe two Xids). > Not sure exactly which Xid(s) yet, and I'm not sure what are the exact > rules, but the Xid becomes a marker th

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-03-23 Thread Alvaro Herrera
I'm coming around to the idea that the fact that you can cancel the wait phase of DETACH CONCURRENTLY creates quite a disaster, and it's not easy to get away from it. The idea that REPEATABLE READ mode means that you now see detached partitions as if they were in normal condition, is completely at

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-03-23 Thread Alvaro Herrera
On 2021-Mar-23, Alvaro Herrera wrote: > So I was about ready to get these patches pushed, when I noticed that in > REPEATABLE READ isolation mode it is possible to insert rows violating > an FK referencing the partition that is being detached. I'm not sure > what is a good solution to this proble

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-03-23 Thread Alvaro Herrera
So I was about ready to get these patches pushed, when I noticed that in REPEATABLE READ isolation mode it is possible to insert rows violating an FK referencing the partition that is being detached. I'm not sure what is a good solution to this problem. The problem goes like this: /* setup */

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-03-21 Thread Justin Pryzby
On Sun, Mar 21, 2021 at 01:14:20PM -0500, Justin Pryzby wrote: > On Sun, Mar 21, 2021 at 03:01:15PM -0300, Alvaro Herrera wrote: > > > But note that it doesn't check if an existing constraint "implies" the new > > > constraint - maybe it should. > > > > Hm, I'm not sure I want to do that, because

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-03-21 Thread Alvaro Herrera
On 2021-Mar-21, Justin Pryzby wrote: > On Sun, Mar 21, 2021 at 03:22:00PM -0300, Alvaro Herrera wrote: > > > So if we do that on DETACH, what would happen on ATTACH? > > Do you mean what happens to the constraint that was already there ? > Nothing, since it's not ours to mess with. Checking Impl

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-03-21 Thread Justin Pryzby
On Sun, Mar 21, 2021 at 03:22:00PM -0300, Alvaro Herrera wrote: > On 2021-Mar-21, Justin Pryzby wrote: > > > On Sun, Mar 21, 2021 at 03:01:15PM -0300, Alvaro Herrera wrote: > > > > But note that it doesn't check if an existing constraint "implies" the > > > > new > > > > constraint - maybe it sho

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-03-21 Thread Alvaro Herrera
On 2021-Mar-21, Justin Pryzby wrote: > On Sun, Mar 21, 2021 at 03:01:15PM -0300, Alvaro Herrera wrote: > > > But note that it doesn't check if an existing constraint "implies" the new > > > constraint - maybe it should. > > > > Hm, I'm not sure I want to do that, because that means that if I late

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-03-21 Thread Justin Pryzby
On Sun, Mar 21, 2021 at 03:01:15PM -0300, Alvaro Herrera wrote: > > But note that it doesn't check if an existing constraint "implies" the new > > constraint - maybe it should. > > Hm, I'm not sure I want to do that, because that means that if I later > have to attach the partition again with the

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-03-21 Thread Alvaro Herrera
On 2021-Mar-19, Alvaro Herrera wrote: > diff --git a/src/backend/utils/cache/partcache.c > b/src/backend/utils/cache/partcache.c > index 0fe4f55b04..6dfa3fb4a8 100644 > --- a/src/backend/utils/cache/partcache.c > +++ b/src/backend/utils/cache/partcache.c > @@ -352,16 +352,9 @@ generate_partition_

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-03-21 Thread Alvaro Herrera
On 2021-Mar-21, Justin Pryzby wrote: > On Fri, Mar 19, 2021 at 10:57:37AM -0300, Alvaro Herrera wrote: > > > Also, it "fails to avoid" adding duplicate constraints: > > > > > > Check constraints: > > > "c" CHECK (i IS NOT NULL AND i > 1 AND i < 2) > > > "cc" CHECK (i IS NOT NULL AND i >=

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-03-21 Thread Justin Pryzby
On Fri, Mar 19, 2021 at 10:57:37AM -0300, Alvaro Herrera wrote: > > Also, it "fails to avoid" adding duplicate constraints: > > > > Check constraints: > > "c" CHECK (i IS NOT NULL AND i > 1 AND i < 2) > > "cc" CHECK (i IS NOT NULL AND i >= 1 AND i < 2) > > "p1_check" CHECK (true) > >

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-03-19 Thread Alvaro Herrera
On 2021-Mar-17, Justin Pryzby wrote: > The v8 patch has the "broken constraint" problem. Yeah, I had misunderstood what the problem was. I think a good solution to this is to have get_partition_parent return the true parent even when a detach is pending, for one particular callsite. (This means

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-03-17 Thread Justin Pryzby
The v8 patch has the "broken constraint" problem. Also, it "fails to avoid" adding duplicate constraints: Check constraints: "c" CHECK (i IS NOT NULL AND i > 1 AND i < 2) "cc" CHECK (i IS NOT NULL AND i >= 1 AND i < 2) "p1_check" CHECK (true) "p1_i_check" CHECK (i IS NOT NULL AND

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-03-17 Thread Alvaro Herrera
On 2021-Mar-15, Alvaro Herrera wrote: > Here's a fixup patch to do it that way. I tried running the commands > you showed and one of them immediately dies with the new error message; > I can't cause the bogus constraint to show up anymore. Actually, that was a silly fix that didn't actually work

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-03-15 Thread Alvaro Herrera
On 2021-Feb-26, Alvaro Herrera wrote: > Hmm, but if we take this approach, then we're still vulnerable to the > problem that somebody can do DETACH CONCURRENTLY and cancel the wait (or > crash the server), then mess up the state before doing DETACH FINALIZE: > when they cancel the wait, the lock w

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-03-11 Thread Alvaro Herrera
Rebase to current sources, to appease CF bot; no other changes. -- Álvaro Herrera39°49'30"S 73°17'W diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index b1de6d0674..ea3ae56991 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sg

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-02-26 Thread Alvaro Herrera
On 2021-Jan-10, Justin Pryzby wrote: > On Fri, Jan 08, 2021 at 04:14:33PM -0300, Alvaro Herrera wrote: > > > > I ended up with apparently broken constraint when running multiple > > > > loops around > > > > a concurrent detach / attach: > > > > > > > > while psql -h /tmp postgres -c "ALTER TABLE

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-01-10 Thread Justin Pryzby
On Fri, Jan 08, 2021 at 04:14:33PM -0300, Alvaro Herrera wrote: > > > 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)T

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2021-01-08 Thread Alvaro Herrera
On 2020-Dec-01, Alvaro Herrera wrote: > On 2020-Nov-30, Justin Pryzby wrote: > Thanks for all the comments. I'll incorporate everything and submit an > updated version later. Here's a rebased version 5, with the typos fixed. More comments below. > > The attname "detached" is a stretch of what'

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-12-25 Thread Andy Fan
On Tue, Aug 4, 2020 at 7:49 AM Alvaro Herrera wrote: > I've been working on the ability to detach a partition from a > partitioned table, without causing blockages to concurrent activity. > I think this operation is critical for some use cases. > > This would be a very great feature. When we can

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-12-01 Thread Alvaro Herrera
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, _(

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-11-30 Thread Justin Pryzby
On Tue, Nov 03, 2020 at 08:56:06PM -0300, Alvaro Herrera wrote: > Here's an updated version of this patch. > > Apart from rebasing to current master, I made the following changes: > > * On the first transaction (the one that marks the partition as > detached), the partition is locked with ShareLo

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-11-30 Thread Alvaro Herrera
On 2020-Nov-30, Anastasia Lubennikova wrote: > The commitfest is nearing the end and this thread is "Waiting on Author". > As far as I see the last message contains a patch. Is there anything left to > work on or it needs review now? Alvaro, are you planning to continue working > on it? Thanks An

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-11-30 Thread Anastasia Lubennikova
On 04.11.2020 02:56, Alvaro Herrera wrote: Here's an updated version of this patch. Apart from rebasing to current master, I made the following changes: * On the first transaction (the one that marks the partition as detached), the partition is locked with ShareLock rather than ShareUpdateExclu

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-11-03 Thread Alvaro Herrera
Here's an updated version of this patch. Apart from rebasing to current master, I made the following changes: * On the first transaction (the one that marks the partition as detached), the partition is locked with ShareLock rather than ShareUpdateExclusiveLock. This means that DML is not allowed

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-10-16 Thread Alvaro Herrera
On 2020-Sep-24, Amit Langote wrote: Hello Amit, > Sorry I totally failed to see the v2 you had posted and a couple of > other emails where you mentioned the issues I brought up. No worries, I appreciate you reviewing this. > However, I am a bit curious about including detached partitions in > s

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-10-14 Thread Andy Fan
Hi David/Alvaro: On Thu, Oct 15, 2020 at 9:09 AM David Rowley wrote: > On Thu, 15 Oct 2020 at 14:04, Andy Fan wrote: > > > > I think if it is possible to implement the detech with a NoWait option . > > > > ALTER TABLE ... DETACH PARTITION .. [NoWait]. > > > > if it can't get the lock, raise "R

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-10-14 Thread David Rowley
On Thu, 15 Oct 2020 at 14:04, Andy Fan wrote: > > I think if it is possible to implement the detech with a NoWait option . > > ALTER TABLE ... DETACH PARTITION .. [NoWait]. > > if it can't get the lock, raise "Resource is Busy" immediately, without > blocking others. > this should be a default b

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-10-14 Thread Alvaro Herrera
On 2020-Oct-15, Andy Fan wrote: > I think if it is possible to implement the detech with a NoWait option . > > ALTER TABLE ... DETACH PARTITION .. [NoWait]. > > if it can't get the lock, raise "Resource is Busy" immediately, > without blocking others. this should be a default behavior. If >

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-10-14 Thread Andy Fan
Hi Alvaro: On Tue, Aug 4, 2020 at 7:49 AM Alvaro Herrera wrote: > I've been working on the ability to detach a partition from a > partitioned table, without causing blockages to concurrent activity. > I think this operation is critical for some use cases. > I think if it is possible to implemen

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-09-30 Thread Michael Paquier
On Thu, Sep 24, 2020 at 12:51:52PM +0900, Amit Langote wrote: > Also, I noticed that looking up a parent's partitions via > RelationBuildPartitionDesc or directly will inspect inhdetached to > include or exclude partitions, but checking if a child table is a > partition of a given parent table via

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-09-23 Thread Amit Langote
Hi Alvaro, Sorry I totally failed to see the v2 you had posted and a couple of other emails where you mentioned the issues I brought up. On Thu, Sep 24, 2020 at 12:23 AM Alvaro Herrera wrote: > On 2020-Sep-23, Amit Langote wrote: > I suspect that we don't really need this defensive constraint.

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-09-23 Thread Alvaro Herrera
On 2020-Sep-23, Amit Langote wrote: Hi Amit, thanks for reviewing this patch! > On Tue, Aug 4, 2020 at 8:49 AM Alvaro Herrera > wrote: > I suspect that we don't really need this defensive constraint. I mean > even after committing the 1st transaction, the partition being > detached still has

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-09-22 Thread Amit Langote
Hi Alvaro, Studying the patch to understand how it works. On Tue, Aug 4, 2020 at 8:49 AM Alvaro Herrera wrote: > Why two transactions? The reason is that in order for this to work, we > make a catalog change (mark it detached), and commit so that all > concurrent transactions can see the change

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-09-11 Thread Robert Haas
On Thu, Sep 10, 2020 at 4:54 PM Alvaro Herrera wrote: > Interesting example, thanks. It seems this can be fixed without > breaking anything else by changing the planner so that it includes > detached partitions when we are in a snapshot-isolation transaction. > Indeed, the results from the detach

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-09-10 Thread Alvaro Herrera
On 2020-Aug-31, Hao Wu wrote: > I tested the patch provided by Alvaro. It seems not well in REPEATABLE READ. > -- the tuples from tpart_2 are gone. > gpadmin=*# select * from tpart; > i | j > + > 10 | 10 > 50 | 50 > (2 rows) Interesting example, thanks. It seems this can be fixed wi

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-09-03 Thread Amit Langote
Hi Hao, On Wed, Sep 2, 2020 at 5:25 PM Hao Wu wrote: > > Not related to DETACH PARTITION, but I found a bug in ATTACH PARTITION. > Here are the steps to reproduce the issue: > > create table tpart(i int, j int) partition by range(i); > create table tpart_1(like tpart); > create table tpart_2(like

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-09-02 Thread Hao Wu
Not related to DETACH PARTITION, but I found a bug in ATTACH PARTITION. Here are the steps to reproduce the issue: create table tpart(i int, j int) partition by range(i); create table tpart_1(like tpart); create table tpart_2(like tpart); create table tpart_default(like tpart);alter table tpart a

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-09-01 Thread Alvaro Herrera
On 2020-Aug-27, Robert Haas wrote: > On Wed, Aug 26, 2020 at 7:40 PM Alvaro Herrera > wrote: > > To mark it detached means to set pg_inherits.inhdetached=true. That > > column name is a bit of a misnomer, since that column really means "is > > in the process of being detached"; the pg_inherits

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-08-31 Thread Hao Wu
the failing row contains (i) = (130). Is this an expected behavior? Regards, Hao Wu From: Robert Haas Sent: Thursday, August 27, 2020 11:46 PM To: Alvaro Herrera Cc: Pg Hackers Subject: Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY On Wed, Aug 26,

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-08-27 Thread Robert Haas
On Wed, Aug 26, 2020 at 7:40 PM Alvaro Herrera wrote: > To mark it detached means to set pg_inherits.inhdetached=true. That > column name is a bit of a misnomer, since that column really means "is > in the process of being detached"; the pg_inherits row goes away > entirely once the detach proces

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-08-26 Thread Alvaro Herrera
On 2020-Aug-04, Robert Haas wrote: > On Mon, Aug 3, 2020 at 7:49 PM Alvaro Herrera > wrote: > > Why two transactions? The reason is that in order for this to work, we > > make a catalog change (mark it detached), and commit so that all > > concurrent transactions can see the change. A second t

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-08-04 Thread Robert Haas
On Mon, Aug 3, 2020 at 7:49 PM Alvaro Herrera wrote: > Why two transactions? The reason is that in order for this to work, we > make a catalog change (mark it detached), and commit so that all > concurrent transactions can see the change. A second transaction waits > for anybody who holds any lo

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-08-04 Thread Alvaro Herrera
On 2020-Aug-03, Alvaro Herrera wrote: > Why two transactions? The reason is that in order for this to work, we > make a catalog change (mark it detached), and commit so that all > concurrent transactions can see the change. A second transaction waits > for anybody who holds any lock on the parti

Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY

2020-08-03 Thread Alvaro Herrera
On 2020-Aug-03, Alvaro Herrera wrote: > There was a lot of great discussion which ended up in Robert completing > a much sought implementation of non-blocking ATTACH. DETACH was > discussed too because it was a goal initially, but eventually dropped > from that patch altogether. Nonetheless, that