Subject: Re: doc: mentioned CREATE+ATTACH PARTITION as an alternative to CREATE TABLE..PARTITION OF

From: Robert Treat <[email-masked]>
Date: Wed, 11 Jan 2023 10:47:54 -0500

On Mon, Sep 5, 2022 at 2:04 PM Justin Pryzby <[email-masked]> wrote:
>
> On Thu, Aug 04, 2022 at 01:45:49AM -0400, Robert Treat wrote:
> > After reading this again, it isn't clear to me that this advice would
> > be more appropriately placed into Section 5.11, aka
> > https://www.postgresql.org/docs/current/ddl-partitioning.html, but in
> > lieu of a specific suggestion for where to place it there (I haven't
> > settled on one yet), IMHO, I think the first sentence of the suggested
> > change should be rewritten as:
> >
> > <para>
> > Note that creating a partition using <literal>PARTITION OF<literal>
> > requires taking an <literal>ACCESS EXCLUSIVE</literal> lock on the parent table.
> > It may be preferable to first CREATE a separate table...
>
> Thanks for looking.  I used your language.
>
> There is some relevant information in ddl.sgml, but not a lot, and it's
> not easily referred to, so I removed the part of the patch that tried to
> cross-reference.
>

Yes, I see now what you are referring to, and thinking maybe an option
would be to also add a reference there back to what will include your
change above.

diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 4b219435d4..c52092a45e 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -4088,7 +4088,9 @@ CREATE TABLE measurement_y2008m02 PARTITION OF measurement
      As an alternative, it is sometimes more convenient to create the
      new table outside the partition structure, and make it a proper
      partition later. This allows new data to be loaded, checked, and
-     transformed prior to it appearing in the partitioned table.
+     transformed prior to it appearing in the partitioned table; see
+     <link linkend="sql-altertable-attach-partition"><literal>ALTER
TABLE ... ATTACH PARTITION</literal></link>
+     for additional details.
      The <literal>CREATE TABLE ... LIKE</literal> option is helpful
      to avoid tediously repeating the parent table's definition:

> @Robert: I wonder why shouldn't CREATE..PARTITION OF *also* be patched
> to first create a table, and then attach the partition, transparently
> doing what everyone would want, without having to re-read the updated
> docs or know to issue two commands?  I wrote a patch for this which
> "doesn't fail tests", but I still wonder if I'm missing something..
>

I was thinking there might be either lock escalation issues or perhaps
issues around index attachment that don't surface using create
partition of, but I don't actually see any, in which case that does
seem like a better change all around. But like you, I feel I must be
overlooking something :-)

> commit 723fa7df82f39aed5d58e5e52ba80caa8cb13515
> Author: Justin Pryzby <[email-masked]>
> Date:   Mon Jul 18 09:24:55 2022 -0500
>
>     doc: mention CREATE+ATTACH PARTITION as an alternative to CREATE TABLE..PARTITION OF
>
>     In v12, 898e5e329 (Allow ATTACH PARTITION with only ShareUpdateExclusiveLock)
>     allows attaching a partition with a weaker lock than in CREATE..PARTITION OF,
>     but it does that silently.  On the one hand, things that are automatically
>     better, without having to enable the option are the best kind of feature.
>
>     OTOH, I doubt many people know to do that, because the docs don't say
>     so, because it was implemented as an transparent improvement.  This
>     patch adds a bit of documentations to make that more visible.
>
>     See also: 898e5e3290a72d288923260143930fb32036c00c
>     Should backpatch to v12
>
> diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
> index 360284e37d6..66138b9299d 100644
> --- a/doc/src/sgml/ddl.sgml
> +++ b/doc/src/sgml/ddl.sgml
> @@ -4092,7 +4092,9 @@ ALTER TABLE measurement ATTACH PARTITION measurement_y2008m02
>
>      <para>
>       The <command>ATTACH PARTITION</command> command requires taking a
> -     <literal>SHARE UPDATE EXCLUSIVE</literal> lock on the partitioned table.
> +     <literal>SHARE UPDATE EXCLUSIVE</literal> lock on the partitioned table,
> +     as opposed to the <literal>Access Exclusive</literal> lock which is
> +     required by <literal>CREATE TABLE .. PARTITION OF</literal>.
>      </para>
>
>      <para>
> diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
> index c14b2010d81..54dbfa72e4c 100644
> --- a/doc/src/sgml/ref/create_table.sgml
> +++ b/doc/src/sgml/ref/create_table.sgml
> @@ -619,6 +619,16 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
>        with <literal>DROP TABLE</literal> requires taking an <literal>ACCESS
>        EXCLUSIVE</literal> lock on the parent table.
>       </para>
> +
> +     <para>
> +      Note that creating a partition using <literal>PARTITION OF<literal>
> +      requires taking an <literal>ACCESS EXCLUSIVE</literal> lock on the parent
> +      table.  It may be preferable to first create a separate table and then
> +      attach it, which does not require as strong a lock.
> +      See <link linkend="sql-altertable-attach-partition">ATTACH PARTITION</link>
> +      for more information.
> +     </para>
> +
>      </listitem>
>     </varlistentry>
>

Robert Treat
https://xzilla.net

---

From: Robert Haas <[email-masked]>
Date: Wed, 11 Jan 2023 16:13:01 -0500

> overlooking something :-)

To be honest, I'm not sure whether either of you are missing anything
or not. I think a major reason why I didn't implement this was that
it's a different code path. DefineRelation() has code to do a bunch of
things that are also done by ATExecAttachPartition(), and I haven't
gone through exhaustively and checked whether there are any relevant
differences. I think that part of the reason that I did not research
that at the time is that the patch was incredibly complicated to get
working at all and I didn't want to take any risk of adding things to
it that might create more problems. Now that it's been a few years, we
might feel more confident.

Another thing that probably deserves at least a bit of thought is the
fact that ATTACH PARTITION just attaches a partition, whereas CREATE
TABLE does a lot more things. Are any of those things potential
hazards? Like what if the newly-created table references the parent
via a foreign key, or uses the parent's row type as a column type or
as part of a column default expression or in a CHECK constraint or
something? Basically, try to think of weird scenarios where the new
table would interact with the parent in some weird way where the
weaker lock would be a problem. Maybe there's nothing to see here: not
sure.

Also, we need to separately analyze the cases where (1) the new
partition is the default partition, (2) the new partition is not the
default partition but a default partition exists, and (3) the new
partition is not the default partition and no default partition
exists.

Sorry not to have more definite thoughts here. I know that when I
developed the original patch, I thought about this case and decided my
brain was full. However, I do not recall whether I knew about any
specific problems that needed to be fixed, or just feared that there
might be some.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

---

From: Robert Treat <[email-masked]>
Date: Thu, 19 Jan 2023 16:47:59 -0500

> specific problems that needed to be fixed, or just feared that there

I think all of that feedback is useful, I guess the immediate question
becomes if Justin wants to try to proceed with his patch implementing
the change, or if adjusting the documentation for the current
implementation is the right move for now.

Robert Treat
https://xzilla.net

---

From: Justin Pryzby <[email-masked]>
Date: Thu, 19 Jan 2023 15:58:32 -0600

> implementation is the right move for now.

The docs change is desirable in any case, since it should be
backpatched, and any patch to change CREATE..PARTITION OF would be for
v17+ anyway.

-- 
Justin

---

From: Jakub Wartak <[email-masked]>
Date: Tue, 14 Mar 2023 15:47:01 +0100

Hi, I've tested the attached patch by Justin and it applied almost
cleanly to the master, but there was a tiny typo and make
postgres-A4.pdf didn't want to run:
Note that creating a partition using <literal>PARTITION OF<literal>
=> (note lack of closing literal) =>
Note that creating a partition using <literal>PARTITION OF</literal>

Attached is version v0002 that contains this fix. @Justin maybe you
could set the status to Ready for Comitter (
https://commitfest.postgresql.org/42/3790/ ) ?

---

From: Tom Lane <[email-masked]>
Date: Thu, 16 Mar 2023 16:52:07 -0400

> backpatched, and any patch to change CREATE..PARTITION OF would be for

Right.  Pushed with a little further effort to align it better with
surrounding text.

			regards, tom lane

---

From: Justin Pryzby <[email-masked]>
Date: Thu, 16 Mar 2023 16:11:59 -0500

> Right.  Pushed with a little further effort to align it better with

Thanks.

      It is possible to use <link linkend="sql-altertable"><command>ALTER
      TABLE ATTACH/DETACH PARTITION</command></link> to perform these
      operations with a weaker lock, thus reducing interference with
      concurrent operations on the partitioned table.

Note that in order for DETACH+DROP to use a lower lock level, it has to be
DETACH CONCURRENTLY.  ATTACH is implicitly uses a lower lock level, but for
DETACH it's only on request.

-- 
Justin

---

From: Tom Lane <[email-masked]>
Date: Thu, 16 Mar 2023 17:27:30 -0400

> DETACH it's only on request.

Right, but that's the sort of detail you should read on that command's man
page, we don't need to duplicate it in N other places.

			regards, tom lane