Thread: Re: doc: alter table references bogus table-specific plannerparameters

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

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

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