Horton

Thread

Commits

  1. Doc: Fix list of storage parameters available for ALTER TABLE

  2. Enhance docs for ALTER TABLE lock levels of storage parms

  3. Reduce lock levels for table storage params related to planning

  4. Reduce lock levels for ALTER TABLE SET autovacuum storage options

  5. Reduce lock levels of some ALTER TABLE cmds

  6. Reduce lock levels of CREATE TRIGGER and some ALTER TABLE, CREATE RULE actions.

  7. Support ALTER TABLESPACE name SET/RESET ( tablespace_options ).

  1. distinguish index cost component from table component

    Justin Pryzby <pryzby@telsasoft.com> — 2020-01-03T14:14:27Z

    Is it possible to tell what component of the cost estimate of an index scan is
from the index reads vs heap ?

It would help to be able to set enable_bitmapscan=FORCE (to make all index
scans go through a bitmap).  Adding OR conditions can sometimes do this.  That
includes cost of bitmap manipulation, but it's good enough for me.

Or maybe explain should report it.

  2. Re: distinguish index cost component from table component

    Jeff Janes <jeff.janes@gmail.com> — 2020-01-03T14:33:35Z

    On Fri, Jan 3, 2020 at 9:14 AM Justin Pryzby <pryzby@telsasoft.com> wrote:

> Is it possible to tell what component of the cost estimate of an index
> scan is
> from the index reads vs heap ?
>

Not that I have found, other than through sprinkling elog statements
throughout the costing code.  Which is horrible, because then you get
estimates for all the considered but rejected index scans as well, but
without the context to know what they are for.  So it only works for toy
queries where there are few possible indexes to consider.

It would help to be able to set enable_bitmapscan=FORCE (to make all index
> scans go through a bitmap).


Doesn't enable_indexscan=off accomplish this already?  It is possible but
not terribly likely to switch from index to seq, rather than from index to
bitmap.  (Unless the index scan was being used to obtain an ordered result,
but a hypothetical enable_bitmapscan=FORCE can't fix that).

Of course this doesn't really answer your question, as the
separately-reported costs of a bitmap heap and bitmap index scan are
unlikely to match what the costs would be of a regular index scan, if they
were reported separately.

Or maybe explain should report it.
>

I wouldn't be optimistic about getting such a backwards-incompatible change
accepted (plus it would surely add some small accounting overhead, which
again would probably not be acceptable). But if you do enough tuning work,
perhaps it would be worth carrying an out-of-tree patch to implement that.
I wouldn't be so interested in writing such a patch, but would be
interested in using one were it available somewhere.

Cheers,

Jeff

  3. Re: distinguish index cost component from table component

    Justin Pryzby <pryzby@telsasoft.com> — 2020-01-03T16:03:15Z

    On Fri, Jan 03, 2020 at 09:33:35AM -0500, Jeff Janes wrote:
> Of course this doesn't really answer your question, as the
> separately-reported costs of a bitmap heap and bitmap index scan are
> unlikely to match what the costs would be of a regular index scan, if they
> were reported separately.

I think the cost of index component of bitmap scan would be exactly the same
as the cost of the original indexscan.

>> Or maybe explain should report it.
> 
> I wouldn't be optimistic about getting such a backwards-incompatible change
> accepted (plus it would surely add some small accounting overhead, which
> again would probably not be acceptable). But if you do enough tuning work,
> perhaps it would be worth carrying an out-of-tree patch to implement that.
> I wouldn't be so interested in writing such a patch, but would be
> interested in using one were it available somewhere.

I did the attached in the simplest possible way.  If it's somehow possible get
the path's index_total_cost from the plan, then there'd be no additional
overhead.

Justin

  4. allow disabling indexscans without disabling bitmapscans

    Justin Pryzby <pryzby@telsasoft.com> — 2020-01-04T16:50:47Z

    Moving to -hackers

I was asking about how to distinguish the index cost component of an indexscan
from the cost of heap.
https://www.postgresql.org/message-id/20200103141427.GK12066%40telsasoft.com

On Fri, Jan 03, 2020 at 09:33:35AM -0500, Jeff Janes wrote:
> > It would help to be able to set enable_bitmapscan=FORCE (to make all index
> > scans go through a bitmap).
> 
> Doesn't enable_indexscan=off accomplish this already?  It is possible but
> not terribly likely to switch from index to seq, rather than from index to
> bitmap.  (Unless the index scan was being used to obtain an ordered result,
> but a hypothetical enable_bitmapscan=FORCE can't fix that).

No, enable_indexscan=off implicitly disables bitmap index scans, since it does:

cost_bitmap_heap_scan():
|startup_cost += indexTotalCost;

But maybe it shouldn't (?)  Or maybe it should take a third value, like
enable_indexscan=bitmaponly, which means what it says.  Actually the name is
confusable with indexonly, so maybe enable_indexscan=bitmap.

A third value isn't really needed anyway; its only utility is that someone
upgrading from v12 who uses enable_indexscan=off (presumably in limited scope)
wouldn't have to also set enable_bitmapscan=off - not a big benefit.

That doesn't affect regress tests at all.

Note, when I tested it, the cost of "bitmap heap scan" was several times higher
than the total cost of indexscan (including heap), even with CPU costs at 0.  I
applied my "bitmap correlation" patch, which seems to gives more reasonable
result.  In any case, the purpose of this patch was primarily diagnostic, and
the heap cost of index scan would be its total cost minus the cost of the
bitmap indexscan node when enable_indexscan=off.  The high cost attributed to
bitmap heapscan is topic for the other patch.

Justin

  5. Re: allow disabling indexscans without disabling bitmapscans

    Justin Pryzby <pryzby@telsasoft.com> — 2020-01-04T19:34:16Z

    On Sat, Jan 04, 2020 at 10:50:47AM -0600, Justin Pryzby wrote:
> > Doesn't enable_indexscan=off accomplish this already?  It is possible but
> > not terribly likely to switch from index to seq, rather than from index to
> > bitmap.  (Unless the index scan was being used to obtain an ordered result,
> > but a hypothetical enable_bitmapscan=FORCE can't fix that).
> 
> No, enable_indexscan=off implicitly disables bitmap index scans, since it does:

I don't know how I went wrong, but the regress tests clued me in..it's as Jeff
said.

Sorry for the noise.

Justin

  6. doc: alter table references bogus table-specific planner parameters

    Justin Pryzby <pryzby@telsasoft.com> — 2020-01-06T02:56:23Z

    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

  7. Re: doc: alter table references bogus table-specific planner parameters

    Simon Riggs <simon@2ndquadrant.com> — 2020-01-06T03:48:52Z

    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

  8. Re: doc: alter table references bogus table-specific planner parameters

    Justin Pryzby <pryzby@telsasoft.com> — 2020-01-06T04:13:14Z

    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

  9. Re: doc: alter table references bogus table-specific planner parameters

    Simon Riggs <simon@2ndquadrant.com> — 2020-01-06T04:33:46Z

    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