Thread: Operator class parameters and sgml docs

Operator class parameters and sgml docs

From
Peter Geoghegan
Date:
Commit 911e7020770 added a variety of new support routines to index
AMs. For example, it added a support function 5 to btree (see
BTOPTIONS_PROC), but didn't document this alongside the other support
functions in btree.sgml.

It looks like the new support functions are fundamentally different to
the existing ones in that they exist only as a way of supplying
parameters to other support functions. The idea was to preserve
compatibility with the old support function signatures. Even still, I
think that the new support functions should get some mention alongside
the older support functions.

I also wonder whether or not xindex.sgml needs to be updated to
account for opclass parameters.

-- 
Peter Geoghegan



Re: Operator class parameters and sgml docs

From
Alexander Korotkov
Date:
Hi, Peter!

On Thu, May 21, 2020 at 12:37 AM Peter Geoghegan <pg@bowt.ie> wrote:
> Commit 911e7020770 added a variety of new support routines to index
> AMs. For example, it added a support function 5 to btree (see
> BTOPTIONS_PROC), but didn't document this alongside the other support
> functions in btree.sgml.
>
> It looks like the new support functions are fundamentally different to
> the existing ones in that they exist only as a way of supplying
> parameters to other support functions. The idea was to preserve
> compatibility with the old support function signatures. Even still, I
> think that the new support functions should get some mention alongside
> the older support functions.
>
> I also wonder whether or not xindex.sgml needs to be updated to
> account for opclass parameters.

Thank you for pointing.  I'm going to take a look on this in next few days.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company



Re: Operator class parameters and sgml docs

From
Alexander Korotkov
Date:
On Thu, May 21, 2020 at 3:17 AM Alexander Korotkov
<a.korotkov@postgrespro.ru> wrote:
>
> On Thu, May 21, 2020 at 12:37 AM Peter Geoghegan <pg@bowt.ie> wrote:
> > Commit 911e7020770 added a variety of new support routines to index
> > AMs. For example, it added a support function 5 to btree (see
> > BTOPTIONS_PROC), but didn't document this alongside the other support
> > functions in btree.sgml.
> >
> > It looks like the new support functions are fundamentally different to
> > the existing ones in that they exist only as a way of supplying
> > parameters to other support functions. The idea was to preserve
> > compatibility with the old support function signatures. Even still, I
> > think that the new support functions should get some mention alongside
> > the older support functions.
> >
> > I also wonder whether or not xindex.sgml needs to be updated to
> > account for opclass parameters.
>
> Thank you for pointing.  I'm going to take a look on this in next few days.


I'm sorry for the delay.  I was very busy with various stuff.  I'm
going to post docs patch next week.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company



Re: Operator class parameters and sgml docs

From
Alexander Korotkov
Date:
On Thu, May 28, 2020 at 11:02 PM Alexander Korotkov
<a.korotkov@postgrespro.ru> wrote:
>
> On Thu, May 21, 2020 at 3:17 AM Alexander Korotkov
> <a.korotkov@postgrespro.ru> wrote:
> >
> > On Thu, May 21, 2020 at 12:37 AM Peter Geoghegan <pg@bowt.ie> wrote:
> > > Commit 911e7020770 added a variety of new support routines to index
> > > AMs. For example, it added a support function 5 to btree (see
> > > BTOPTIONS_PROC), but didn't document this alongside the other support
> > > functions in btree.sgml.
> > >
> > > It looks like the new support functions are fundamentally different to
> > > the existing ones in that they exist only as a way of supplying
> > > parameters to other support functions. The idea was to preserve
> > > compatibility with the old support function signatures. Even still, I
> > > think that the new support functions should get some mention alongside
> > > the older support functions.
> > >
> > > I also wonder whether or not xindex.sgml needs to be updated to
> > > account for opclass parameters.
> >
> > Thank you for pointing.  I'm going to take a look on this in next few days.
>
> I'm sorry for the delay.  I was very busy with various stuff.  I'm
> going to post docs patch next week.


Thank you for patience.  The documentation patch is attached.  I think
it requires review by native english speaker.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Attachment

Re: Operator class parameters and sgml docs

From
Peter Geoghegan
Date:
On Tue, Jun 16, 2020 at 4:24 AM Alexander Korotkov
<a.korotkov@postgrespro.ru> wrote:
> Thank you for patience.  The documentation patch is attached.  I think
> it requires review by native english speaker.

* "...paramaters that controls" should be "...paramaters that control".

* "with set of operator class specific option" should be "with a set
of operator class specific options".

* "The options could be accessible from each support function" should
be "The options can be accessed from other support functions"

(At least I think that that's what you meant)

It's very hard to write documentation like this, even for native
English speakers. I think that it's important to have something in
place, though. The GiST example helps a lot.

-- 
Peter Geoghegan



Re: Operator class parameters and sgml docs

From
Alexander Korotkov
Date:
On Wed, Jun 17, 2020 at 2:50 AM Peter Geoghegan <pg@bowt.ie> wrote:
> On Tue, Jun 16, 2020 at 4:24 AM Alexander Korotkov
> <a.korotkov@postgrespro.ru> wrote:
> > Thank you for patience.  The documentation patch is attached.  I think
> > it requires review by native english speaker.
>
> * "...paramaters that controls" should be "...paramaters that control".
>
> * "with set of operator class specific option" should be "with a set
> of operator class specific options".
>
> * "The options could be accessible from each support function" should
> be "The options can be accessed from other support functions"

Fixed, thanks!

> It's very hard to write documentation like this, even for native
> English speakers. I think that it's important to have something in
> place, though. The GiST example helps a lot.

I've added a complete example for defining a set of parameters and
accessing them from another support function to the GiST
documentation.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Attachment

Re: Operator class parameters and sgml docs

From
Alexander Korotkov
Date:
On Wed, Jun 17, 2020 at 2:00 PM Alexander Korotkov
<a.korotkov@postgrespro.ru> wrote:
> On Wed, Jun 17, 2020 at 2:50 AM Peter Geoghegan <pg@bowt.ie> wrote:
> > On Tue, Jun 16, 2020 at 4:24 AM Alexander Korotkov
> > <a.korotkov@postgrespro.ru> wrote:
> > > Thank you for patience.  The documentation patch is attached.  I think
> > > it requires review by native english speaker.
> >
> > * "...paramaters that controls" should be "...paramaters that control".
> >
> > * "with set of operator class specific option" should be "with a set
> > of operator class specific options".
> >
> > * "The options could be accessible from each support function" should
> > be "The options can be accessed from other support functions"
>
> Fixed, thanks!
>
> > It's very hard to write documentation like this, even for native
> > English speakers. I think that it's important to have something in
> > place, though. The GiST example helps a lot.
>
> I've added a complete example for defining a set of parameters and
> accessing them from another support function to the GiST
> documentation.

I'm going to push this patch if there are no objections.  I'm almost
sure that documentation of opclass options will require further
adjustments.  However, I think the current patch makes it better, not
worse.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company



Re: Operator class parameters and sgml docs

From
Alexander Korotkov
Date:
On Thu, Jun 18, 2020 at 8:06 PM Alexander Korotkov
<a.korotkov@postgrespro.ru> wrote:
> On Wed, Jun 17, 2020 at 2:00 PM Alexander Korotkov
> <a.korotkov@postgrespro.ru> wrote:
> > On Wed, Jun 17, 2020 at 2:50 AM Peter Geoghegan <pg@bowt.ie> wrote:
> > > On Tue, Jun 16, 2020 at 4:24 AM Alexander Korotkov
> > > <a.korotkov@postgrespro.ru> wrote:
> > > > Thank you for patience.  The documentation patch is attached.  I think
> > > > it requires review by native english speaker.
> > >
> > > * "...paramaters that controls" should be "...paramaters that control".
> > >
> > > * "with set of operator class specific option" should be "with a set
> > > of operator class specific options".
> > >
> > > * "The options could be accessible from each support function" should
> > > be "The options can be accessed from other support functions"
> >
> > Fixed, thanks!
> >
> > > It's very hard to write documentation like this, even for native
> > > English speakers. I think that it's important to have something in
> > > place, though. The GiST example helps a lot.
> >
> > I've added a complete example for defining a set of parameters and
> > accessing them from another support function to the GiST
> > documentation.
>
> I'm going to push this patch if there are no objections.  I'm almost
> sure that documentation of opclass options will require further
> adjustments.  However, I think the current patch makes it better, not
> worse.

So, pushed!

------
Regards,
Alexander Korotkov



Re: Operator class parameters and sgml docs

From
Peter Geoghegan
Date:
On Sat, Jun 20, 2020 at 3:55 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> So, pushed!

Noticed one small thing. You forgot to update this part from the B-Tree docs:

"As shown in Table 37.9, btree defines one required and three optional
support functions. The four user-defined methods are:"

Thanks
-- 
Peter Geoghegan



Re: Operator class parameters and sgml docs

From
Alexander Korotkov
Date:
On Sat, Jun 20, 2020 at 10:15 PM Peter Geoghegan <pg@bowt.ie> wrote:
> On Sat, Jun 20, 2020 at 3:55 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> > So, pushed!
>
> Noticed one small thing. You forgot to update this part from the B-Tree docs:
>
> "As shown in Table 37.9, btree defines one required and three optional
> support functions. The four user-defined methods are:"

Thanks!  I've also spotted a similar issue in SP-GiST.  Fix for both is pushed.

------
Regards,
Alexander Korotkov



Re: Operator class parameters and sgml docs

From
Justin Pryzby
Date:
On Sat, Jun 20, 2020 at 01:55:33PM +0300, Alexander Korotkov wrote:
> On Thu, Jun 18, 2020 at 8:06 PM Alexander Korotkov <a.korotkov@postgrespro.ru> wrote:
> > On Wed, Jun 17, 2020 at 2:00 PM Alexander Korotkov <a.korotkov@postgrespro.ru> wrote:
> > > On Wed, Jun 17, 2020 at 2:50 AM Peter Geoghegan <pg@bowt.ie> wrote:
> > > > On Tue, Jun 16, 2020 at 4:24 AM Alexander Korotkov <a.korotkov@postgrespro.ru> wrote:
> > > > > Thank you for patience.  The documentation patch is attached.  I think
> > > > > it requires review by native english speaker.
...
> > > Fixed, thanks!
> > >
> > > > It's very hard to write documentation like this, even for native
> > > > English speakers. I think that it's important to have something in
> > > > place, though. The GiST example helps a lot.
...
> > I'm going to push this patch if there are no objections.  I'm almost
> > sure that documentation of opclass options will require further
> > adjustments.  However, I think the current patch makes it better, not
> > worse.
> 
> So, pushed!

Find attached some language review of user-facing docs.

diff --git a/doc/src/sgml/brin.sgml b/doc/src/sgml/brin.sgml
index d7f1af7819..4c5eeb875f 100644
--- a/doc/src/sgml/brin.sgml
+++ b/doc/src/sgml/brin.sgml
@@ -562,7 +562,7 @@ typedef struct BrinOpcInfo
   </varlistentry>
  </variablelist>

  [-Optionally, an-]{+An+} operator class for <acronym>BRIN</acronym> can [-supply-]{+optionally specify+} the
  following method:

  <variablelist>
@@ -570,22 +570,22 @@ typedef struct BrinOpcInfo
     <term><function>void options(local_relopts *relopts)</function></term>
     <listitem>
      <para>
       Defines {+a+} set of user-visible parameters that control operator class
       behavior.
      </para>

      <para>
       The <function>options</function> function [-has given-]{+is passed a+} pointer to {+a+}
       <replaceable>local_relopts</replaceable> struct, which needs to be
       filled with a set of operator class specific options.  The options
       can be accessed from other support functions using {+the+}
       <literal>PG_HAS_OPCLASS_OPTIONS()</literal> and
       <literal>PG_GET_OPCLASS_OPTIONS()</literal> macros.
      </para>

      <para>
       Since both key extraction [-for-]{+of+} indexed [-value-]{+values+} and representation of the
       key in <acronym>GIN</acronym> are flexible, [-it-]{+they+} may [-depends-]{+depend+} on
       user-specified parameters.
      </para>
     </listitem>
diff --git a/doc/src/sgml/btree.sgml b/doc/src/sgml/btree.sgml
index 2c4dd48ea3..b17b166e84 100644
--- a/doc/src/sgml/btree.sgml
+++ b/doc/src/sgml/btree.sgml
@@ -557,7 +557,7 @@ equalimage(<replaceable>opcintype</replaceable> <type>oid</type>) returns bool
     Optionally, a B-tree operator family may provide
     <function>options</function> (<quote>operator class specific
     options</quote>) support functions, registered under support
     function number 5.  These functions define {+a+} set of user-visible
     parameters that control operator class behavior.
    </para>
    <para>
@@ -566,19 +566,19 @@ equalimage(<replaceable>opcintype</replaceable> <type>oid</type>) returns bool
<synopsis>
options(<replaceable>relopts</replaceable> <type>local_relopts *</type>) returns void
</synopsis>
     The function [-has given-]{+is passed a+} pointer to {+a+} <replaceable>local_relopts</replaceable>
     struct, which needs to be filled with a set of operator class
     specific options.  The options can be accessed from other support
     functions using {+the+} <literal>PG_HAS_OPCLASS_OPTIONS()</literal> and
     <literal>PG_GET_OPCLASS_OPTIONS()</literal> macros.
    </para>
    <para>
     Currently, no B-Tree operator class has {+an+} <function>options</function>
     support function.  B-tree doesn't allow flexible representation of keys
     like GiST, SP-GiST, GIN and BRIN do.  So, <function>options</function>
     probably doesn't have much [-usage-]{+application+} in {+the+} current[-shape of-] B-tree index
     access method.  Nevertheless, this support function was added to B-tree
     for uniformity, and[-probably it-] will [-found its usage-]{+probably find uses+} during further
     evolution of B-tree in <productname>PostgreSQL</productname>.
    </para>
   </listitem>
diff --git a/doc/src/sgml/gin.sgml b/doc/src/sgml/gin.sgml
index d85e7c8796..7a8c18a449 100644
--- a/doc/src/sgml/gin.sgml
+++ b/doc/src/sgml/gin.sgml
@@ -411,17 +411,17 @@
      </para>

      <para>
       The <function>options</function> function [-has given-]{+is passed a+} pointer to {+a+}
       <replaceable>local_relopts</replaceable> struct, which needs to be
       filled with [-s-]{+a+} set of operator class specific options.  The options
       can be accessed from other support functions using {+the+}
       <literal>PG_HAS_OPCLASS_OPTIONS()</literal> and
       <literal>PG_GET_OPCLASS_OPTIONS()</literal> macros.
      </para>

      <para>
       Since both key extraction [-for-]{+of+} indexed [-value-]{+values+} and representation of the
       key in <acronym>GIN</acronym> are flexible, [-it-]{+they+} may [-depends-]{+depend+} on
       user-specified parameters.
      </para>
     </listitem>
diff --git a/doc/src/sgml/gist.sgml b/doc/src/sgml/gist.sgml
index 31c28fdb61..5d970ee9f2 100644
--- a/doc/src/sgml/gist.sgml
+++ b/doc/src/sgml/gist.sgml
@@ -946,7 +946,7 @@ my_fetch(PG_FUNCTION_ARGS)
     <term><function>options</function></term>
     <listitem>
      <para>
       Allows [-defintion-]{+definition+} of user-visible parameters that control operator
       class behavior.
      </para>

@@ -962,16 +962,16 @@ LANGUAGE C STRICT;
      </para>

      <para>
       The function [-has given-]{+is passed a+} pointer to {+a+} <replaceable>local_relopts</replaceable>
       struct, which needs to be filled with a set of operator class
       specific options.  The options can be accessed from other support
       functions using {+the+} <literal>PG_HAS_OPCLASS_OPTIONS()</literal> and
       <literal>PG_GET_OPCLASS_OPTIONS()</literal> macros.
      </para>

       <para>
        [-The sample-]{+An example+} implementation of [-my_option()-]{+my_options()+} and parameters [-usage-]
[-        in the another-]{+use+}
{+        from other+} support [-function-]{+functions+} are given below:

<programlisting>
typedef enum MyEnumType
@@ -990,7 +990,7 @@ typedef struct
    int     str_param;  /* string parameter */
} MyOptionsStruct;

/* String [-representations for-]{+representation of+} enum values */
static relopt_enum_elt_def myEnumValues[] =
{
    {"on", MY_ENUM_ON},
@@ -1002,7 +1002,7 @@ static relopt_enum_elt_def myEnumValues[] =
static char *str_param_default = "default";

/*
 * Sample [-validatior:-]{+validator:+} checks that string is not longer than 8 bytes.
 */
static void 
validate_my_string_relopt(const char *value)
@@ -1090,8 +1090,8 @@ my_compress(PG_FUNCTION_ARGS)

      <para>
       Since the representation of the key in <acronym>GiST</acronym> is
       flexible, it may [-depends-]{+depend+} on user-specified parameters.  For [-instace,-]{+instance,+}
       the length of key signature may be [-such parameter.-]{+specified.+}  See
       <literal>gtsvector_options()</literal> for example.
      </para>
     </listitem>
diff --git a/doc/src/sgml/spgist.sgml b/doc/src/sgml/spgist.sgml
index 03f914735b..1395dbaf88 100644
--- a/doc/src/sgml/spgist.sgml
+++ b/doc/src/sgml/spgist.sgml
@@ -895,16 +895,16 @@ LANGUAGE C STRICT;
      </para>

      <para>
       The function [-has given-]{+is passed a+} pointer to {+a+} <replaceable>local_relopts</replaceable>
       struct, which needs to be filled with a set of operator class
       specific options.  The options can be accessed from other support
       functions using {+the+} <literal>PG_HAS_OPCLASS_OPTIONS()</literal> and
       <literal>PG_GET_OPCLASS_OPTIONS()</literal> macros.
      </para>

      <para>
       Since the representation of the key in <acronym>SP-GiST</acronym> is
       flexible, it may [-depends-]{+depend+} on user-specified parameters.
      </para>
     </listitem>
    </varlistentry>
diff --git a/doc/src/sgml/xindex.sgml b/doc/src/sgml/xindex.sgml
index 0e4587a81b..2cfd71b5b7 100644
--- a/doc/src/sgml/xindex.sgml
+++ b/doc/src/sgml/xindex.sgml
@@ -410,9 +410,9 @@
  </para>

  <para>
   Additionally, some opclasses allow [-user-]{+users+} to [-set specific parameters,-]{+specify parameters+} which
   [-controls its-]{+control their+} behavior.  Each builtin index access method [-have-]{+has an+} optional
   <function>options</function> support function, which defines {+a+} set of
   opclass-specific parameters.
  </para>

@@ -459,7 +459,7 @@
      </row>
      <row>
       <entry>
        Defines {+a+} set of options that are specific [-for-]{+to+} this operator class
        (optional)
       </entry>
       <entry>5</entry>
@@ -501,7 +501,7 @@
      </row>
      <row>
       <entry>
        Defines {+a+} set of options that are specific [-for-]{+to+} this operator class
        (optional)
       </entry>
       <entry>3</entry>
@@ -584,7 +584,7 @@
      <row>
       <entry><function>options</function></entry>
       <entry>
        Defines {+a+} set of options that are specific [-for-]{+to+} this operator class
        (optional)
       </entry>
       <entry>10</entry>
@@ -643,7 +643,7 @@
      <row>
       <entry><function>options</function></entry>
       <entry>
        Defines {+a+} set of options that are specific [-for-]{+to+} this operator class
        (optional)
       </entry>
       <entry>6</entry>
@@ -720,7 +720,7 @@
      <row>
       <entry><function>options</function></entry>
       <entry>
        Defines {+a+} set of options that are specific [-for-]{+to+} this operator class
        (optional)
       </entry>
       <entry>7</entry>
@@ -778,7 +778,7 @@
      <row>
       <entry><function>options</function></entry>
       <entry>
        Defines {+a+} set of options that are specific [-for-]{+to+} this operator class
        (optional)
       </entry>
       <entry>5</entry>

Attachment

Re: Operator class parameters and sgml docs

From
Justin Pryzby
Date:
And a couple more in spgist.sgml (some of which were not added by this patch).

Attachment

Re: Operator class parameters and sgml docs

From
Alexander Korotkov
Date:
On Sun, Jun 21, 2020 at 2:28 AM Justin Pryzby <pryzby@telsasoft.com> wrote:
> And a couple more in spgist.sgml (some of which were not added by this patch).

Pushed, thanks!

------
Regards,
Alexander Korotkov