public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v36 6/6] Index skip scan documentation
4+ messages / 4 participants
[nested] [flat]

* [PATCH v36 6/6] Index skip scan documentation
@ 2020-06-08 18:45  Dmitrii Dolgov <[email protected]>
  0 siblings, 0 replies; 4+ messages in thread

From: Dmitrii Dolgov @ 2020-06-08 18:45 UTC (permalink / raw)

Covers added GUC and the extenion of index am interface.

Author: Jesper Pedersen, Dmitry Dolgov
Reviewed-by: Thomas Munro, David Rowley, Floris Van Nee, Kyotaro Horiguchi, Tomas Vondra, Peter Geoghegan
---
 doc/src/sgml/config.sgml  | 15 ++++++++++
 doc/src/sgml/indexam.sgml | 62 +++++++++++++++++++++++++++++++++++++++
 doc/src/sgml/indices.sgml | 23 +++++++++++++++
 3 files changed, 100 insertions(+)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 6ce5907896..7b78325813 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -4654,6 +4654,21 @@ ANY <replaceable class="parameter">num_sync</replaceable> ( <replaceable class="
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-enable-indexskipscan" xreflabel="enable_indexskipscan">
+      <term><varname>enable_indexskipscan</varname> (<type>boolean</type>)
+      <indexterm>
+       <primary><varname>enable_indexskipscan</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Enables or disables the query planner's use of index-skip-scan plan
+        types (see <xref linkend="indexes-index-skip-scans"/>). The default is
+        <literal>on</literal>.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-enable-material" xreflabel="enable_material">
       <term><varname>enable_material</varname> (<type>boolean</type>)
       <indexterm>
diff --git a/doc/src/sgml/indexam.sgml b/doc/src/sgml/indexam.sgml
index 8c552c22bd..b3ca91211f 100644
--- a/doc/src/sgml/indexam.sgml
+++ b/doc/src/sgml/indexam.sgml
@@ -694,6 +694,68 @@ amrestrpos (IndexScanDesc scan);
 
   <para>
 <programlisting>
+bool
+amskip (IndexScanDesc scan,
+        ScanDirection direction,
+        ScanDirection indexdir,
+        bool scanstart,
+        int prefix);
+</programlisting>
+  Skip past all tuples where the first 'prefix' columns have the same value as
+  the last tuple returned in the current scan. The arguments are:
+
+   <variablelist>
+    <varlistentry>
+     <term><parameter>scan</parameter></term>
+     <listitem>
+      <para>
+       Index scan information
+      </para>
+     </listitem>
+    </varlistentry>
+
+    <varlistentry>
+     <term><parameter>direction</parameter></term>
+     <listitem>
+      <para>
+       The direction in which data is advancing.
+      </para>
+     </listitem>
+    </varlistentry>
+
+    <varlistentry>
+     <term><parameter>indexdir</parameter></term>
+     <listitem>
+      <para>
+        The index direction, in which data must be read.
+      </para>
+     </listitem>
+    </varlistentry>
+
+    <varlistentry>
+     <term><parameter>scanstart</parameter></term>
+     <listitem>
+      <para>
+        Whether or not it is a start of the scan.
+      </para>
+     </listitem>
+    </varlistentry>
+
+    <varlistentry>
+     <term><parameter>prefix</parameter></term>
+     <listitem>
+      <para>
+        Distinct prefix size.
+      </para>
+     </listitem>
+    </varlistentry>
+
+   </variablelist>
+
+  </para>
+
+  <para>
+<programlisting>
 Size
 amestimateparallelscan (void);
 </programlisting>
diff --git a/doc/src/sgml/indices.sgml b/doc/src/sgml/indices.sgml
index 28adaba72d..1cf8550e0c 100644
--- a/doc/src/sgml/indices.sgml
+++ b/doc/src/sgml/indices.sgml
@@ -1281,6 +1281,29 @@ SELECT target FROM tests WHERE subject = 'some-subject' AND success;
    and later will recognize such cases and allow index-only scans to be
    generated, but older versions will not.
   </para>
+
+  <sect2 id="indexes-index-skip-scans">
+    <title>Index Skip Scans</title>
+
+    <indexterm zone="indexes-index-skip-scans">
+      <primary>index</primary>
+      <secondary>index-skip scans</secondary>
+    </indexterm>
+    <indexterm zone="indexes-index-skip-scans">
+      <primary>index-skip scan</primary>
+    </indexterm>
+
+    <para>
+     When the rows retrieved from an index scan are then deduplicated by
+     eliminating rows matching on a prefix of index keys (e.g. when using
+     <literal>SELECT DISTINCT</literal>), the planner will consider
+     skipping groups of rows with a matching key prefix. When a row with
+     a particular prefix is found, remaining rows with the same key prefix
+     are skipped.  The larger the number of rows with the same key prefix
+     rows (i.e. the lower the number of distinct key prefixes in the index),
+     the more efficient this is.
+    </para>
+  </sect2>
  </sect1>
 
 
-- 
2.21.0


--op7mf2i72xk5yu7c--





^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: Clear padding in PgStat_HashKey keys
@ 2024-11-04 07:25  Michael Paquier <[email protected]>
  0 siblings, 1 reply; 4+ messages in thread

From: Michael Paquier @ 2024-11-04 07:25 UTC (permalink / raw)
  To: Bertrand Drouvot <[email protected]>; +Cc: [email protected]

On Sun, Nov 03, 2024 at 04:25:41AM +0000, Bertrand Drouvot wrote:
> We are using sizeof(PgStat_HashKey) in pgstat_cmp_hash_key() and we compute the
> hash hash key in pgstat_hash_hash_key() using the PgStat_HashKey struct size as
> input: this lead to unexpected results if the keys contain random data in the 
> padding bytes.

So you've seen that your patch was behaving weirdly once you have
added padding because the hash key size has been extended, leading to
relfilenode entries not being fetched when they should, right?

Perhaps it would be simpler to use a {0} like anywhere else for
PgStat_HashKey in pgstat_fetch_entry() and pgstat_drop_entry(), then
initialize the individual fields?
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: Clear padding in PgStat_HashKey keys
@ 2024-11-04 08:27  Jelte Fennema-Nio <[email protected]>
  parent: Michael Paquier <[email protected]>
  0 siblings, 1 reply; 4+ messages in thread

From: Jelte Fennema-Nio @ 2024-11-04 08:27 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: Bertrand Drouvot <[email protected]>; [email protected]

On Mon, 4 Nov 2024 at 08:25, Michael Paquier <[email protected]> wrote:
> Perhaps it would be simpler to use a {0} like anywhere else for
> PgStat_HashKey in pgstat_fetch_entry() and pgstat_drop_entry(), then
> initialize the individual fields?

{0} doesn't clear padding, it only sets all the fields to 0.

So in many places we use memset or MemSet to clear the padding already:

rg 'memset.*key' -S | wc -l
31

And even using memset in this manner isn't a standards compliant way
of handling this problem[0]. But it seems to have worked well enough
in practice.

Whether it's worth changing this now or only when we actually
introduce padding is another question though. But the code itself and
reasoning is sensible imo.

[0]: https://www.postgresql.org/message-id/flat/8c1e502a337e9557278f31abf877c321%40anastigmatix.net#41132...






^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: Clear padding in PgStat_HashKey keys
@ 2024-11-04 08:48  Bertrand Drouvot <[email protected]>
  parent: Jelte Fennema-Nio <[email protected]>
  0 siblings, 0 replies; 4+ messages in thread

From: Bertrand Drouvot @ 2024-11-04 08:48 UTC (permalink / raw)
  To: Jelte Fennema-Nio <[email protected]>; +Cc: Michael Paquier <[email protected]>; [email protected]

Hi,

On Mon, Nov 04, 2024 at 09:27:43AM +0100, Jelte Fennema-Nio wrote:
> On Mon, 4 Nov 2024 at 08:25, Michael Paquier <[email protected]> wrote:
> > Perhaps it would be simpler to use a {0} like anywhere else for
> > PgStat_HashKey in pgstat_fetch_entry() and pgstat_drop_entry(), then
> > initialize the individual fields?
> 
> {0} doesn't clear padding, it only sets all the fields to 0.

Thank you both to look at it!

> 
> So in many places we use memset or MemSet to clear the padding already:
> 
> rg 'memset.*key' -S | wc -l
> 31

Yeah, and 9fd45870c1 did not touch some of them (like the one I mentioned up-thread
in LOCALLOCKTAG localtag in LockHeldByMe()).

> And even using memset in this manner isn't a standards compliant way
> of handling this problem[0]. But it seems to have worked well enough
> in practice.
> 
> Whether it's worth changing this now or only when we actually
> introduce padding is another question though.

I think it's worth to do it now:

1/ for example, it's done in LOCALLOCKTAG localtag in LockHeldByMe() while
LOCALLOCKTAG does not contain padding (see up-thread).

2/ the one that will add padding could miss this thread and spend some time
to figure out why his patch does break existing stats.

3/ when dealing with structs that are used as hash key I think we should not wait
for padding to be introduced.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com






^ permalink  raw  reply  [nested|flat] 4+ messages in thread


end of thread, other threads:[~2024-11-04 08:48 UTC | newest]

Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-06-08 18:45 [PATCH v36 6/6] Index skip scan documentation Dmitrii Dolgov <[email protected]>
2024-11-04 07:25 Re: Clear padding in PgStat_HashKey keys Michael Paquier <[email protected]>
2024-11-04 08:27 ` Re: Clear padding in PgStat_HashKey keys Jelte Fennema-Nio <[email protected]>
2024-11-04 08:48   ` Re: Clear padding in PgStat_HashKey keys Bertrand Drouvot <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox