Thread: Operator class parameters and sgml docs
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
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
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
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
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
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
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
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
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
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
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
And a couple more in spgist.sgml (some of which were not added by this patch).
Attachment
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