pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feeddoc: alter table references bogus table-specific planner parameters
7+ messages / 3 participants
[nested] [flat]
Commits from this thread:
2020-01-24 6de7bcb76f c4c76d198e b9988facae b9a9cb1bfd Doc: Fix list of storage parameters available for ALTER TABLE
Related threads (commitfest entry [doc] alter table references bogus table-specific planner parameters/2417):
distinguish index cost component from table component
* doc: alter table references bogus table-specific planner parameters
@ 2020-01-06 02:56 Justin Pryzby <pryzby@telsasoft.com>
0 siblings, 1 reply; 7+ messages in thread
From: Justin Pryzby @ 2020-01-06 02:56 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org; +Cc: Simon Riggs <simon@2ndQuadrant.com>
commit 6f3a13ff058f15d565a30c16c0c2cb14cc994e42 Enhance docs for ALTER TABLE lock levels of storage parms
Author: Simon Riggs <simon@2ndQuadrant.com>
Date: Mon Mar 6 16:48:12 2017 +0530
<varlistentry>
<term><literal>SET ( <replaceable class="PARAMETER">storage_parameter</replaceable> = <replaceable class="PARAMETER">value</replaceable> [, ... ] )</literal></term>
...
- Changing fillfactor and autovacuum storage parameters acquires a <literal>SHARE UPDATE EXCLUSIVE</literal> lock.
+ <literal>SHARE UPDATE EXCLUSIVE</literal> lock will be taken for
+ fillfactor and autovacuum storage parameters, as well as the
+ following planner related parameters:
+ effective_io_concurrency, parallel_workers, seq_page_cost
+ random_page_cost, n_distinct and n_distinct_inherited.
effective_io_concurrency, seq_page_cost and random_page_cost cannot be set for
a table - reloptions.c shows that they've always been RELOPT_KIND_TABLESPACE.
n_distinct lock mode seems to have been changed and documented at e5550d5f ;
21d4e2e2 claimed to do the same, but the LOCKMODE is never used.
See also:
commit 21d4e2e20656381b4652eb675af4f6d65053607f Reduce lock levels for table storage params related to planning
Author: Simon Riggs <simon@2ndQuadrant.com>
Date: Mon Mar 6 16:04:31 2017 +0530
commit 47167b7907a802ed39b179c8780b76359468f076 Reduce lock levels for ALTER TABLE SET autovacuum storage options
Author: Simon Riggs <simon@2ndQuadrant.com>
Date: Fri Aug 14 14:19:28 2015 +0100
commit e5550d5fec66aa74caad1f79b79826ec64898688 Reduce lock levels of some ALTER TABLE cmds
Author: Simon Riggs <simon@2ndQuadrant.com>
Date: Sun Apr 6 11:13:43 2014 -0400
commit 2dbbda02e7e688311e161a912a0ce00cde9bb6fc Reduce lock levels of CREATE TRIGGER and some ALTER TABLE, CREATE RULE actions.
Author: Simon Riggs <simon@2ndQuadrant.com>
Date: Wed Jul 28 05:22:24 2010 +0000
commit d86d51a95810caebcea587498068ff32fe28293e Support ALTER TABLESPACE name SET/RESET ( tablespace_options ).
Author: Robert Haas <rhaas@postgresql.org>
Date: Tue Jan 5 21:54:00 2010 +0000
Justin
Attachments:
v1-0001-Fixes-for-commit-6f3a13ff.patch (text/x-diff, 2.5K) download
From 64699ee90ef6ebe9459e3b2b1f603f30ec2c49c8 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sun, 5 Jan 2020 19:39:29 -0600
Subject: [PATCH v1] Fixes for commit 6f3a13ff
Should backpatch to v10.
---
doc/src/sgml/ref/alter_table.sgml | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index bb1e48a..e5f39c2 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -676,32 +676,30 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
</varlistentry>
<varlistentry>
<term><literal>SET ( <replaceable class="parameter">storage_parameter</replaceable> = <replaceable class="parameter">value</replaceable> [, ... ] )</literal></term>
<listitem>
<para>
- This form changes one or more storage parameters for the table. See
+ This form changes one or more storage or planner parameters for the table. See
<xref linkend="sql-createtable-storage-parameters"
endterm="sql-createtable-storage-parameters-title"/>
for details on the available parameters. Note that the table contents
- will not be modified immediately by this command; depending on the
+ will not be modified immediately by setting its storage parameters; depending on the
parameter you might need to rewrite the table to get the desired effects.
That can be done with <link linkend="sql-vacuum">VACUUM
FULL</link>, <xref linkend="sql-cluster"/> or one of the forms
of <command>ALTER TABLE</command> that forces a table rewrite.
For planner related parameters, changes will take effect from the next
time the table is locked so currently executing queries will not be
affected.
</para>
<para>
<literal>SHARE UPDATE EXCLUSIVE</literal> lock will be taken for
fillfactor, toast and autovacuum storage parameters, as well as the
- following planner related parameters:
- <varname>effective_io_concurrency</varname>, <varname>parallel_workers</varname>, <varname>seq_page_cost</varname>,
- <varname>random_page_cost</varname>, <varname>n_distinct</varname> and <varname>n_distinct_inherited</varname>.
+ <varname>parallel_workers</varname> planner parameter.
</para>
</listitem>
</varlistentry>
<varlistentry>
<term><literal>RESET ( <replaceable class="parameter">storage_parameter</replaceable> [, ... ] )</literal></term>
--
2.7.4
* Re: doc: alter table references bogus table-specific planner parameters
@ 2020-01-06 03:48 Simon Riggs <simon@2ndquadrant.com>
parent: Justin Pryzby <pryzby@telsasoft.com>
0 siblings, 1 reply; 7+ messages in thread
From: Simon Riggs @ 2020-01-06 03:48 UTC (permalink / raw)
To: Justin Pryzby <pryzby@telsasoft.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Mon, 6 Jan 2020 at 02:56, Justin Pryzby <pryzby@telsasoft.com> wrote:
> commit 6f3a13ff058f15d565a30c16c0c2cb14cc994e42 Enhance docs for ALTER
> TABLE lock levels of storage parms
> Author: Simon Riggs <simon@2ndQuadrant.com>
> Date: Mon Mar 6 16:48:12 2017 +0530
>
> <varlistentry>
> <term><literal>SET ( <replaceable
> class="PARAMETER">storage_parameter</replaceable> = <replaceable
> class="PARAMETER">value</replaceable> [, ... ] )</literal></term>
> ...
> - Changing fillfactor and autovacuum storage parameters acquires a
> <literal>SHARE UPDATE EXCLUSIVE</literal> lock.
> + <literal>SHARE UPDATE EXCLUSIVE</literal> lock will be taken for
> + fillfactor and autovacuum storage parameters, as well as the
> + following planner related parameters:
> + effective_io_concurrency, parallel_workers, seq_page_cost
> + random_page_cost, n_distinct and n_distinct_inherited.
>
> effective_io_concurrency, seq_page_cost and random_page_cost cannot be set
> for
> a table - reloptions.c shows that they've always been
> RELOPT_KIND_TABLESPACE.
>
Right, but if they were settable at table-level, the lock levels shown
would be accurate.
I agree with the sentiment of the third doc change, but your patch removes
the mention of n_distinct, which isn't appropriate.
The second change in your patch alters the meaning of the sentence in a way
that is counter to the first change. The name of these parameters is
"Storage Parameters" (in various places); I might agree with describing
them in text as "storage or planner parameters", but if you do that you
can't then just refer to "storage parameters" later, because if you do it
implies that planner parameters operate differently to storage parameters,
which they don't.
> n_distinct lock mode seems to have been changed and documented at e5550d5f
> ;
> 21d4e2e2 claimed to do the same, but the LOCKMODE is never used.
>
But neither does it need to because we don't lock tablespaces.
Thanks for your comments.
--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/;
PostgreSQL Solutions for the Enterprise
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: doc: alter table references bogus table-specific planner parameters
@ 2020-01-06 04:13 Justin Pryzby <pryzby@telsasoft.com>
parent: Simon Riggs <simon@2ndquadrant.com>
0 siblings, 1 reply; 7+ messages in thread
From: Justin Pryzby @ 2020-01-06 04:13 UTC (permalink / raw)
To: Simon Riggs <simon@2ndquadrant.com>; +Cc: pgsql-hackers@lists.postgresql.org
On Mon, Jan 06, 2020 at 03:48:52AM +0000, Simon Riggs wrote:
> On Mon, 6 Jan 2020 at 02:56, Justin Pryzby <pryzby@telsasoft.com> wrote:
>
> > commit 6f3a13ff058f15d565a30c16c0c2cb14cc994e42 Enhance docs for ALTER TABLE lock levels of storage parms
> > Author: Simon Riggs <simon@2ndQuadrant.com>
> > Date: Mon Mar 6 16:48:12 2017 +0530
> >
> > <varlistentry>
> > <term><literal>SET ( <replaceable
> > class="PARAMETER">storage_parameter</replaceable> = <replaceable
> > class="PARAMETER">value</replaceable> [, ... ] )</literal></term>
> > ...
> > - Changing fillfactor and autovacuum storage parameters acquires a
> > <literal>SHARE UPDATE EXCLUSIVE</literal> lock.
> > + <literal>SHARE UPDATE EXCLUSIVE</literal> lock will be taken for
> > + fillfactor and autovacuum storage parameters, as well as the
> > + following planner related parameters:
> > + effective_io_concurrency, parallel_workers, seq_page_cost
> > + random_page_cost, n_distinct and n_distinct_inherited.
> >
> > effective_io_concurrency, seq_page_cost and random_page_cost cannot be set
> > for
> > a table - reloptions.c shows that they've always been
> > RELOPT_KIND_TABLESPACE.
>
> I agree with the sentiment of the third doc change, but your patch removes
> the mention of n_distinct, which isn't appropriate.
I think it's correct to remove n_distinct there, as it's documented previously,
since e5550d5f. That's a per-attribute option (not storage) and can't be
specified there.
<varlistentry>
<term><literal>SET ( <replaceable class="PARAMETER">attribute_option</replaceable> = <replaceable class="PARAMETER">value</replaceable> [, ... ] )</literal></term>
<term><literal>RESET ( <replaceable class="PARAMETER">attribute_option</replaceable> [, ... ] )</literal></term>
<listitem>
<para>
This form sets or resets per-attribute options. Currently, the only
...
+ <para>
+ Changing per-attribute options acquires a
+ <literal>SHARE UPDATE EXCLUSIVE</literal> lock.
+ </para>
> The second change in your patch alters the meaning of the sentence in a way
> that is counter to the first change. The name of these parameters is
> "Storage Parameters" (in various places); I might agree with describing
> them in text as "storage or planner parameters", but if you do that you
> can't then just refer to "storage parameters" later, because if you do it
> implies that planner parameters operate differently to storage parameters,
> which they don't.
The 2nd change is:
for details on the available parameters. Note that the table contents
- will not be modified immediately by this command; depending on the
+ will not be modified immediately by setting its storage parameters; depending on the
parameter you might need to rewrite the table to get the desired effects.
I deliberately qualified that as referring only to "storage params" rather than
"this command", since planner params never "modify the table contents".
Possibly other instances in the document (and createtable) should be changed
for consistency.
Justin
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: doc: alter table references bogus table-specific planner parameters
@ 2020-01-06 04:33 Simon Riggs <simon@2ndquadrant.com>
parent: Justin Pryzby <pryzby@telsasoft.com>
0 siblings, 0 replies; 7+ messages in thread
From: Simon Riggs @ 2020-01-06 04:33 UTC (permalink / raw)
To: Justin Pryzby <pryzby@telsasoft.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Mon, 6 Jan 2020 at 04:13, Justin Pryzby <pryzby@telsasoft.com> wrote:
>
> > I agree with the sentiment of the third doc change, but your patch
> removes
> > the mention of n_distinct, which isn't appropriate.
>
> I think it's correct to remove n_distinct there, as it's documented
> previously,
> since e5550d5f. That's a per-attribute option (not storage) and can't be
> specified there.
>
OK, then agreed.
> The second change in your patch alters the meaning of the sentence in a
> way
> > that is counter to the first change. The name of these parameters is
> > "Storage Parameters" (in various places); I might agree with describing
> > them in text as "storage or planner parameters", but if you do that you
> > can't then just refer to "storage parameters" later, because if you do it
> > implies that planner parameters operate differently to storage
> parameters,
> > which they don't.
>
> The 2nd change is:
>
> for details on the available parameters. Note that the table
> contents
> - will not be modified immediately by this command; depending on the
> + will not be modified immediately by setting its storage parameters;
> depending on the
> parameter you might need to rewrite the table to get the desired
> effects.
>
> I deliberately qualified that as referring only to "storage params" rather
> than
> "this command", since planner params never "modify the table contents".
> Possibly other instances in the document (and createtable) should be
> changed
> for consistency.
>
Yes, but it's not a correction, just a different preference of wording.
--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/;
PostgreSQL Solutions for the Enterprise
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: doc: alter table references bogus table-specific planner parameters
@ 2020-01-22 01:27 Justin Pryzby <pryzby@telsasoft.com>
0 siblings, 1 reply; 7+ messages in thread
From: Justin Pryzby @ 2020-01-22 01:27 UTC (permalink / raw)
To: Simon Riggs <simon@2ndquadrant.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Mon, Jan 06, 2020 at 04:33:46AM +0000, Simon Riggs wrote:
> On Mon, 6 Jan 2020 at 04:13, Justin Pryzby <pryzby@telsasoft.com> wrote:
> > > I agree with the sentiment of the third doc change, but your patch removes
> > > the mention of n_distinct, which isn't appropriate.
> >
> > I think it's correct to remove n_distinct there, as it's documented previously,
> > since e5550d5f. That's a per-attribute option (not storage) and can't be
> > specified there.
>
> OK, then agreed.
Attached minimal patch with just this hunk.
https://commitfest.postgresql.org/27/2417/
=> RFC
Justin
(I'm resending in a new thread since it looks like the first message was
somehow sent as a reply to an unrelated thread.)
Attachments:
v2-0001-doc-alter-table-references-bogus-table-specific-p.patch (text/x-diff, 1.3K) download
From 23873bbf32740b0f78f2102eb615e6a6aa615b8c Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sun, 5 Jan 2020 19:39:29 -0600
Subject: [PATCH v2] [doc] alter table references bogus table-specific planner
parameters
https://commitfest.postgresql.org/27/2417/
Fixes for commit 6f3a13ff
Should backpatch to v10.
---
doc/src/sgml/ref/alter_table.sgml | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index bae6e6a..67eddfb 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -714,9 +714,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
<para>
<literal>SHARE UPDATE EXCLUSIVE</literal> lock will be taken for
fillfactor, toast and autovacuum storage parameters, as well as the
- following planner related parameters:
- <varname>effective_io_concurrency</varname>, <varname>parallel_workers</varname>, <varname>seq_page_cost</varname>,
- <varname>random_page_cost</varname>, <varname>n_distinct</varname> and <varname>n_distinct_inherited</varname>.
+ <varname>parallel_workers</varname> planner parameter.
</para>
</listitem>
</varlistentry>
--
2.7.4
* Re: doc: alter table references bogus table-specific planner parameters
@ 2020-01-22 04:53 Michael Paquier <michael@paquier.xyz>
parent: Justin Pryzby <pryzby@telsasoft.com>
0 siblings, 1 reply; 7+ messages in thread
From: Michael Paquier @ 2020-01-22 04:53 UTC (permalink / raw)
To: Justin Pryzby <pryzby@telsasoft.com>; +Cc: Simon Riggs <simon@2ndquadrant.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Tue, Jan 21, 2020 at 07:27:47PM -0600, Justin Pryzby wrote:
> Attached minimal patch with just this hunk.
>
> https://commitfest.postgresql.org/27/2417/
> => RFC
I think that you should be more careful when you think that you create
a new thread. On my client for example, I can see that this message
is part of its last thread and still holds references to the previous
thread. My guess is that as you are using gmail you just changed the
subject, thinking that it actually created a new thread.
> @@ -714,9 +714,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
> <para>
> <literal>SHARE UPDATE EXCLUSIVE</literal> lock will be taken for
> fillfactor, toast and autovacuum storage parameters, as well as the
> - following planner related parameters:
> - <varname>effective_io_concurrency</varname>, <varname>parallel_workers</varname>, <varname>seq_page_cost</varname>,
> - <varname>random_page_cost</varname>, <varname>n_distinct</varname> and <varname>n_distinct_inherited</varname>.
> + <varname>parallel_workers</varname> planner parameter.
Right. n_distinct_inherited and n_distinct can only be set for
attribute, and the docs are clear about that.
effective_io_concurrency, seq_page_cost and random_page_cost apply to
a tablespace. There is one other thing that this this paragraph
misses though: vacuum_index_cleanup is a parameter dedicated to
vacuum, and not autovacuum. So to be clear I think that the first
sentence should mention "vacuum" as much as "autovacuum" in the list
of storage parameter types impacted by the lower-level lock taken.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: doc: alter table references bogus table-specific planner parameters
@ 2020-01-24 00:59 Michael Paquier <michael@paquier.xyz>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 7+ messages in thread
From: Michael Paquier @ 2020-01-24 00:59 UTC (permalink / raw)
To: Justin Pryzby <pryzby@telsasoft.com>; +Cc: Simon Riggs <simon@2ndquadrant.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Wed, Jan 22, 2020 at 01:53:32PM +0900, Michael Paquier wrote:
>> @@ -714,9 +714,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
>> <para>
>> <literal>SHARE UPDATE EXCLUSIVE</literal> lock will be taken for
>> fillfactor, toast and autovacuum storage parameters, as well as the
>> - following planner related parameters:
>> - <varname>effective_io_concurrency</varname>, <varname>parallel_workers</varname>, <varname>seq_page_cost</varname>,
>> - <varname>random_page_cost</varname>, <varname>n_distinct</varname> and <varname>n_distinct_inherited</varname>.
>> + <varname>parallel_workers</varname> planner parameter.
>
> Right. n_distinct_inherited and n_distinct can only be set for
> attribute, and the docs are clear about that.
Okay, fixed that list and backpatched down to 10.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2020-01-24 00:59 UTC | newest]
Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-01-06 02:56 doc: alter table references bogus table-specific planner parameters Justin Pryzby <pryzby@telsasoft.com>
2020-01-06 03:48 ` Simon Riggs <simon@2ndquadrant.com>
2020-01-06 04:13 ` Justin Pryzby <pryzby@telsasoft.com>
2020-01-06 04:33 ` Simon Riggs <simon@2ndquadrant.com>
2020-01-22 01:27 Re: doc: alter table references bogus table-specific planner parameters Justin Pryzby <pryzby@telsasoft.com>
2020-01-22 04:53 ` Michael Paquier <michael@paquier.xyz>
2020-01-24 00:59 ` Michael Paquier <michael@paquier.xyz>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox