Обсуждение: doc: mentioned CREATE+ATTACH PARTITION as an alternative to CREATE TABLE..PARTITION OF
doc: mentioned CREATE+ATTACH PARTITION as an alternative to CREATE TABLE..PARTITION OF
От
Justin Pryzby
Дата:
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>
Re: doc: mentioned CREATE+ATTACH PARTITION as an alternative to CREATE TABLE..PARTITION OF
От
Andrew Dunstan
Дата:
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
Re: doc: mentioned CREATE+ATTACH PARTITION as an alternative to CREATE TABLE..PARTITION OF
От
Justin Pryzby
Дата:
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
Re: doc: mentioned CREATE+ATTACH PARTITION as an alternative to CREATE TABLE..PARTITION OF
От
Justin Pryzby
Дата:
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 > >
Вложения
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
Re: doc: mentioned CREATE+ATTACH PARTITION as an alternative to CREATE TABLE..PARTITION OF
От
Justin Pryzby
Дата:
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