Date: Wed, 17 Mar 2021 13:45:34 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Amit Langote <amitlangote09@gmail.com>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY
Message-ID: <20210317184534.GB11765@telsasoft.com>
References: <20210315230437.GA20801@alvherre.pgsql>
 <20210317174843.GA29968@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210317174843.GA29968@alvherre.pgsql>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20210317184534.GB11765%40telsasoft.com>
Precedence: bulk

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 i >= 1 AND i < 2)

> diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
> index 5c9f4af1d5..0cb846f408 100644
> --- a/doc/src/sgml/catalogs.sgml
> +++ b/doc/src/sgml/catalogs.sgml
> @@ -4485,6 +4485,16 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
>         when using declarative partitioning.
>        </para></entry>
>       </row>
> +
> +     <row>
> +      <entry role="catalog_table_entry"><para role="column_definition">
> +       <structfield>inhdetachpending</structfield> <type>bool</type>
> +      </para>
> +      <para>
> +       Set to true for a partition that is in the process of being detached;
> +       false otherwise.
> +      </para></entry>
> +     </row>

Remove "Set to" ?
And say <literal>true</literal> and <literal>false</literal>

Probably you'll hate the suggestion, but maybe it should be "pendingdetach".
We already have pg_settings.pending_restart.

> +      If <literal>CONCURRENTLY</literal> is specified, this process runs in two
> +      transactions in order to avoid blocking other sessions that might be accessing
> +      the partitioned table.  During the first transaction, a
> +      <literal>SHARE UPDATE EXCLUSIVE</literal> lock is taken on both parent table and
> +      partition, and its partition is marked detached; at that point, the transaction
> +      is committed and all transactions using the partitioned table are waited for.
> +      Once all those transactions are gone, the second stage acquires

Instead of "gone", say "have completed" ?

> +/*
> + * MarkInheritDetached
> + *
> + * When a partition is detached from its parent concurrently, we don't
> + * remove the pg_inherits row until a second transaction; as a preparatory
> + * step, this function marks the entry as 'detached', so that other

*pending detached

> + * The strategy for concurrency is to first modify the partition catalog
> + * rows to make it visible to everyone that the partition is detached,

the inherits catalog?

> +	/*
> +	 * In concurrent mode, the partition is locked with share-update-exclusive
> +	 * in the first transaction.  This allows concurrent transactions to be
> +	 * doing DML to the partition.

> +	/*
> +	 * Check inheritance conditions and either delete the pg_inherits row
> +	 * (in non-concurrent mode) or just set the inhisdetached flag.

detachpending