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

It's easy to use CREATE TABLE..LIKE + ALTER..ATTACH PARTITION to avoid
acquiring a strong lock when creating a new partition.
But it's also easy to forget.

commit 76c0d1198cf2908423b321cd3340d296cb668c8e
Author: Justin Pryzby <pryzbyj@telsasoft.com>
Date:   Mon Jul 18 09:24:55 2022 -0500

    doc: mention CREATE+ATTACH PARTITION as an alternative to CREATE TABLE..PARTITION OF
    
    See also: 898e5e3290a72d288923260143930fb32036c00c
    Should backpatch to v12

diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index 6bbf15ed1a4..db7d8710bae 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 acquires 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 of a lock.
+      See <link linkend="sql-altertable-attach-partition">ATTACH PARTITION</link>
+      and <xref linkend="ddl-partitioning"/> for more information.
+     </para>
+
     </listitem>
    </varlistentry>
 



On 2022-07-18 Mo 10:33, Justin Pryzby wrote:
> It's easy to use CREATE TABLE..LIKE + ALTER..ATTACH PARTITION to avoid
> acquiring a strong lock when creating a new partition.
> But it's also easy to forget.
>
> commit 76c0d1198cf2908423b321cd3340d296cb668c8e
> Author: Justin Pryzby <pryzbyj@telsasoft.com>
> Date:   Mon Jul 18 09:24:55 2022 -0500
>
>     doc: mention CREATE+ATTACH PARTITION as an alternative to CREATE TABLE..PARTITION OF
>     
>     See also: 898e5e3290a72d288923260143930fb32036c00c
>     Should backpatch to v12
>
> diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
> index 6bbf15ed1a4..db7d8710bae 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 acquires 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 of a lock.
> +      See <link linkend="sql-altertable-attach-partition">ATTACH PARTITION</link>
> +      and <xref linkend="ddl-partitioning"/> for more information.
> +     </para>
> +
>      </listitem>
>     </varlistentry>
>  


Style nitpick.


I would prefer "does not require as strong a lock."


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com




On Mon, Jul 18, 2022 at 10:39 AM Andrew Dunstan <andrew@dunslane.net> wrote:
> On 2022-07-18 Mo 10:33, Justin Pryzby wrote:
> > It's easy to use CREATE TABLE..LIKE + ALTER..ATTACH PARTITION to avoid
> > acquiring a strong lock when creating a new partition.
> > But it's also easy to forget.
> >
> > commit 76c0d1198cf2908423b321cd3340d296cb668c8e
> > Author: Justin Pryzby <pryzbyj@telsasoft.com>
> > Date:   Mon Jul 18 09:24:55 2022 -0500
> >
> >     doc: mention CREATE+ATTACH PARTITION as an alternative to CREATE TABLE..PARTITION OF
> >
> >     See also: 898e5e3290a72d288923260143930fb32036c00c
> >     Should backpatch to v12
> >
> > diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
> > index 6bbf15ed1a4..db7d8710bae 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 acquires 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 of a lock.
> > +      See <link linkend="sql-altertable-attach-partition">ATTACH PARTITION</link>
> > +      and <xref linkend="ddl-partitioning"/> for more information.
> > +     </para>
> > +
> >      </listitem>
> >     </varlistentry>
> >
>
> Style nitpick.
>
> I would prefer "does not require as strong a lock."
>
FWIW, this is also proper grammar as well.

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...


Robert Treat
https://xzilla.net



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.

@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..

commit 723fa7df82f39aed5d58e5e52ba80caa8cb13515
Author: Justin Pryzby <pryzbyj@telsasoft.com>
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>
 



On Mon, Sep 5, 2022 at 2:04 PM Justin Pryzby <pryzby@telsasoft.com> 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 <pryzbyj@telsasoft.com>
> 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



On Wed, Jan 11, 2023 at 10:48 AM Robert Treat <rob@xzilla.net> wrote:
> > @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 :-)

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



On Wed, Jan 11, 2023 at 4:13 PM Robert Haas <robertmhaas@gmail.com> wrote:
> On Wed, Jan 11, 2023 at 10:48 AM Robert Treat <rob@xzilla.net> wrote:
> > > @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 :-)
>
> 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.
>

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



On Thu, Jan 19, 2023 at 04:47:59PM -0500, Robert Treat wrote:
> 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.

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



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/ ) ?

On Thu, Jan 19, 2023 at 10:58 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
>
> On Thu, Jan 19, 2023 at 04:47:59PM -0500, Robert Treat wrote:
> > 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.
>
> 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
>
>

Attachment
Justin Pryzby <pryzby@telsasoft.com> writes:
> On Thu, Jan 19, 2023 at 04:47:59PM -0500, Robert Treat wrote:
>> 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.

> 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.

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

            regards, tom lane



On Thu, Mar 16, 2023 at 04:52:07PM -0400, Tom Lane wrote:
> Justin Pryzby <pryzby@telsasoft.com> writes:
> > On Thu, Jan 19, 2023 at 04:47:59PM -0500, Robert Treat wrote:
> >> 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.
> 
> > 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.
> 
> Right.  Pushed with a little further effort to align it better with
> surrounding text.

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



Justin Pryzby <pryzby@telsasoft.com> writes:
> On Thu, Mar 16, 2023 at 04:52:07PM -0400, Tom Lane wrote:
>       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.

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