Document "59.2. Built-in Operator Classes" have a clerical error?

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
28 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Document "59.2. Built-in Operator Classes" have a clerical error?

osdba


hi all:

In Document "Table 59-1. Built-in GiST Operator Classes":

"range_ops any range type && &> &< >> << <@ -|- = @> @>", exist double "@>",
 
Should be "<@>" ?




 

Reply | Threaded
Open this post in threaded view
|

Re: Document "59.2. Built-in Operator Classes" have a clerical error?

Michael Paquier-2
On Mon, Aug 03, 2020 at 03:14:56PM +0800, osdba wrote:
> "range_opsany range type&& &> &< >> << <@ -|- = @> @>", exist double "@>",
>  
> Should be "<@ @>" ?

Indeed, this needs to be improved.  Another issue on the same page is
that point_ops lists the same operator three times, <@.  Other index
pages don't seem to have any inconsistencies, fortunately.
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Document "59.2. Built-in Operator Classes" have a clerical error?

Tom Lane-2
Michael Paquier <[hidden email]> writes:
> On Mon, Aug 03, 2020 at 03:14:56PM +0800, osdba wrote:
>> "range_opsany range type&& &> &< >> << <@ -|- = @> @>", exist double "@>",
>> Should be "<@ @>" ?

> Indeed, this needs to be improved.  Another issue on the same page is
> that point_ops lists the same operator three times, <@.  Other index
> pages don't seem to have any inconsistencies, fortunately.

psql's handy new \dAo query clarifies things a bit:

regression=# \dAo gist range_ops
                List of operators of operator families
  AM  | Operator family |        Operator         | Strategy | Purpose
------+-----------------+-------------------------+----------+---------
 gist | range_ops       | <<(anyrange,anyrange)   |        1 | search
 gist | range_ops       | &<(anyrange,anyrange)   |        2 | search
 gist | range_ops       | &&(anyrange,anyrange)   |        3 | search
 gist | range_ops       | &>(anyrange,anyrange)   |        4 | search
 gist | range_ops       | >>(anyrange,anyrange)   |        5 | search
 gist | range_ops       | -|-(anyrange,anyrange)  |        6 | search
 gist | range_ops       | @>(anyrange,anyrange)   |        7 | search
 gist | range_ops       | <@(anyrange,anyrange)   |        8 | search
 gist | range_ops       | =(anyrange,anyrange)    |       18 | search
 gist | range_ops       | @>(anyrange,anyelement) |       16 | search
(10 rows)

regression=# \dAo gist point_ops
              List of operators of operator families
  AM  | Operator family |     Operator      | Strategy | Purpose  
------+-----------------+-------------------+----------+----------
 gist | point_ops       | <<(point,point)   |        1 | search
 gist | point_ops       | >>(point,point)   |        5 | search
 gist | point_ops       | ~=(point,point)   |        6 | search
 gist | point_ops       | <^(point,point)   |       10 | search
 gist | point_ops       | >^(point,point)   |       11 | search
 gist | point_ops       | <->(point,point)  |       15 | ordering
 gist | point_ops       | <@(point,box)     |       28 | search
 gist | point_ops       | <@(point,circle)  |       68 | search
 gist | point_ops       | <@(point,polygon) |       48 | search
(9 rows)

So I don't think it's a clerical error, but certainly showing these
operators this way is none too helpful.  Perhaps including the input types
in this table (and its siblings elsewhere) would be a good thing.

(Looking at these, I'm reminded anew that the sort ordering used by \dAo
is still questionable ...)

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: Document "59.2. Built-in Operator Classes" have a clerical error?

Michael Paquier-2
On Mon, Aug 03, 2020 at 07:23:48AM -0400, Tom Lane wrote:
> So I don't think it's a clerical error, but certainly showing these
> operators this way is none too helpful.  Perhaps including the input types
> in this table (and its siblings elsewhere) would be a good thing.

Ah, thanks.  I did not consider the psql shortcut.  I agree that
including the input types would be a good idea.  But just doing that
may not be enough IMO as the character policy used on our website for
the HTML docs makes that harder to parse.  What if we switched the
third column to have one line per operator with its input type, and
use morerows to show the full set of indexable operators one opclass
is associated with?  Contrary to wait events, those tables don't
change much.  I am wondering as well if we should consider mentioning
\dAo on all those pages, say at the bottom of each table listing the
opclasses.
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Document "59.2. Built-in Operator Classes" have a clerical error?

Bruce Momjian
In reply to this post by Michael Paquier-2
On Mon, Aug  3, 2020 at 04:49:04PM +0900, Michael Paquier wrote:
> On Mon, Aug 03, 2020 at 03:14:56PM +0800, osdba wrote:
> > "range_opsany range type&& &> &< >> << <@ -|- = @> @>", exist double "@>",
> >  
> > Should be "<@ @>" ?
>
> Indeed, this needs to be improved.  Another issue on the same page is
> that point_ops lists the same operator three times, <@.  Other index
> pages don't seem to have any inconsistencies, fortunately.

I decided to remove the duplicates and just add "(multiple)" after
operators that have multiple system table entries;  patch attached.

--
  Bruce Momjian  <[hidden email]>        https://momjian.us
  EnterpriseDB                             https://enterprisedb.com

  The usefulness of a cup is in its emptiness, Bruce Lee


operator.diff (797 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Document "59.2. Built-in Operator Classes" have a clerical error?

Michael Paquier-2
On Fri, Aug 21, 2020 at 07:16:16PM -0400, Bruce Momjian wrote:
> I decided to remove the duplicates and just add "(multiple)" after
> operators that have multiple system table entries;  patch attached.

Sounds like a good idea to me.  Thanks.

Another idea I had after reading your email would be to use "for
multiple types", but simpler is likely better.
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Document "59.2. Built-in Operator Classes" have a clerical error?

Alvaro Herrera-9
In reply to this post by Bruce Momjian
On 2020-Aug-21, Bruce Momjian wrote:

> On Mon, Aug  3, 2020 at 04:49:04PM +0900, Michael Paquier wrote:
> > On Mon, Aug 03, 2020 at 03:14:56PM +0800, osdba wrote:
> > > "range_opsany range type&& &> &< >> << <@ -|- = @> @>", exist double "@>",
> > >  
> > > Should be "<@ @>" ?
> >
> > Indeed, this needs to be improved.  Another issue on the same page is
> > that point_ops lists the same operator three times, <@.  Other index
> > pages don't seem to have any inconsistencies, fortunately.
>
> I decided to remove the duplicates and just add "(multiple)" after
> operators that have multiple system table entries;  patch attached.

Now that we can point people to psql's \dAo, do we really need to have
these tables at all?


--
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Reply | Threaded
Open this post in threaded view
|

Re: Document "59.2. Built-in Operator Classes" have a clerical error?

Michael Paquier-2
On Fri, Aug 21, 2020 at 11:19:07PM -0400, Alvaro Herrera wrote:
> Now that we can point people to psql's \dAo, do we really need to have
> these tables at all?

Having the tables is IMO still useful as a quick reference for users
that don't have immediately psql at hand when working on an
application, for example imagine somebody using pgAdmin.  And I would
imagine that such people are not few.  Another risk here is somebody
using psql with a server that has a major version different than the
one they are working on, leading to false assumptions?  Having at
least a mention to those psql shortcuts in the docs is still a good
idea IMO, as said upthread:
https://www.postgresql.org/message-id/20200804055651.GC2091@...
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Document "59.2. Built-in Operator Classes" have a clerical error?

Bruce Momjian
On Sat, Aug 22, 2020 at 09:23:19PM +0900, Michael Paquier wrote:

> On Fri, Aug 21, 2020 at 11:19:07PM -0400, Alvaro Herrera wrote:
> > Now that we can point people to psql's \dAo, do we really need to have
> > these tables at all?
>
> Having the tables is IMO still useful as a quick reference for users
> that don't have immediately psql at hand when working on an
> application, for example imagine somebody using pgAdmin.  And I would
> imagine that such people are not few.  Another risk here is somebody
> using psql with a server that has a major version different than the
> one they are working on, leading to false assumptions?  Having at
> least a mention to those psql shortcuts in the docs is still a good
> idea IMO, as said upthread:
> https://www.postgresql.org/message-id/20200804055651.GC2091@...

Yeah, I kind of like the table myself too, because this topic is already
so complicated.

--
  Bruce Momjian  <[hidden email]>        https://momjian.us
  EnterpriseDB                             https://enterprisedb.com

  The usefulness of a cup is in its emptiness, Bruce Lee



Reply | Threaded
Open this post in threaded view
|

Re: Document "59.2. Built-in Operator Classes" have a clerical error?

Tom Lane-2
Bruce Momjian <[hidden email]> writes:
> Yeah, I kind of like the table myself too, because this topic is already
> so complicated.

Agreed.  I'm not very happy with the suggestion of "(multiple)" though;
I think that will just add confusion.

If you don't want to go all the way and list the operators with their
input types, maybe we should just do what the OP thought was correct
and delete the duplicate operator names.  It's already the case that
the table isn't telling you exactly which input types the operators
accept, so why not be a little bit fuzzier?

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: Document "59.2. Built-in Operator Classes" have a clerical error?

Alvaro Herrera-9
On 2020-Aug-22, Tom Lane wrote:

> If you don't want to go all the way and list the operators with their
> input types, maybe we should just do what the OP thought was correct
> and delete the duplicate operator names.  It's already the case that
> the table isn't telling you exactly which input types the operators
> accept, so why not be a little bit fuzzier?

Well, if we're going to have a table, let's have a useful table.  What's
wrong with using the same contents \dAo shows? It seemed reasonable
enough to me.

Now of course I would suggest that other client programs such as pgAdmin
ought to display what \dAo shows too ;-)

--
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Reply | Threaded
Open this post in threaded view
|

Re: Document "59.2. Built-in Operator Classes" have a clerical error?

Bruce Momjian
On Sat, Aug 22, 2020 at 02:59:02PM -0400, Alvaro Herrera wrote:

> On 2020-Aug-22, Tom Lane wrote:
>
> > If you don't want to go all the way and list the operators with their
> > input types, maybe we should just do what the OP thought was correct
> > and delete the duplicate operator names.  It's already the case that
> > the table isn't telling you exactly which input types the operators
> > accept, so why not be a little bit fuzzier?
>
> Well, if we're going to have a table, let's have a useful table.  What's
> wrong with using the same contents \dAo shows? It seemed reasonable
> enough to me.

I don't think it is worth it, plus we would need to adjust the docs if
system catalog layout changes.  I think we just want something concise
and simple, but also accurate.

--
  Bruce Momjian  <[hidden email]>        https://momjian.us
  EnterpriseDB                             https://enterprisedb.com

  The usefulness of a cup is in its emptiness, Bruce Lee



Reply | Threaded
Open this post in threaded view
|

Re: Document "59.2. Built-in Operator Classes" have a clerical error?

Alvaro Herrera-9
On 2020-Aug-24, Bruce Momjian wrote:

> I don't think it is worth it, plus we would need to adjust the docs if
> system catalog layout changes.  I think we just want something concise
> and simple, but also accurate.

I argued for \dAo, not straight catalog output -- that was a straw man.

I can't produce the docbook right now but I volunteer to show a
screenshot for what I propose later this week.

--
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Reply | Threaded
Open this post in threaded view
|

Re: Document "59.2. Built-in Operator Classes" have a clerical error?

Bruce Momjian
On Mon, Aug 24, 2020 at 11:59:22AM -0400, Alvaro Herrera wrote:

> On 2020-Aug-24, Bruce Momjian wrote:
>
> > I don't think it is worth it, plus we would need to adjust the docs if
> > system catalog layout changes.  I think we just want something concise
> > and simple, but also accurate.
>
> I argued for \dAo, not straight catalog output -- that was a straw man.
>
> I can't produce the docbook right now but I volunteer to show a
> screenshot for what I propose later this week.

Sure, I can wait.  Is this the only place where it would make sense?

--
  Bruce Momjian  <[hidden email]>        https://momjian.us
  EnterpriseDB                             https://enterprisedb.com

  The usefulness of a cup is in its emptiness, Bruce Lee



Reply | Threaded
Open this post in threaded view
|

Re: Document "59.2. Built-in Operator Classes" have a clerical error?

Michael Paquier-2
On Mon, Aug 24, 2020 at 12:01:00PM -0400, Bruce Momjian wrote:
> Sure, I can wait.  Is this the only place where it would make sense?

I think so.  If there are other places, it does not prevent improving
what we already know needs improvement.

FWIW, the layout I was thinking about is something like the patch
attached.  I have only patched GIN to give an idea of the shape of the
tables.  The PNG file attached is a screenshot of the HTML generated.
I know that we try to limit the use of morerows, but it seems much
better to me to use morerows for those pages here knowing the small
size of the tables.  We could split that into multiple tables instead,
still I find the single-table approach cleaner.
--
Michael

gin_ops_doc.png (109K) Download Attachment
doc-gin-opr-table.patch (3K) Download Attachment
signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Document "59.2. Built-in Operator Classes" have a clerical error?

Bruce Momjian
On Tue, Aug 25, 2020 at 01:44:03PM +0900, Michael Paquier wrote:
> On Mon, Aug 24, 2020 at 12:01:00PM -0400, Bruce Momjian wrote:
> > Sure, I can wait.  Is this the only place where it would make sense?
>
> I think so.  If there are other places, it does not prevent improving
> what we already know needs improvement.

Well, my point is that if this area looks different from other places,
it could look odd, so we can't always make incremental fixes to the
docs, though it might be fine in this case.

--
  Bruce Momjian  <[hidden email]>        https://momjian.us
  EnterpriseDB                             https://enterprisedb.com

  The usefulness of a cup is in its emptiness, Bruce Lee



Reply | Threaded
Open this post in threaded view
|

Re: Document "59.2. Built-in Operator Classes" have a clerical error?

Alvaro Herrera-9
In reply to this post by Michael Paquier-2
On 2020-Aug-25, Michael Paquier wrote:

> I think so.  If there are other places, it does not prevent improving
> what we already know needs improvement.
>
> FWIW, the layout I was thinking about is something like the patch
> attached.

This looks to me enough of an improvement that I +1 it, and yes this is
what I was imagining also.

(With the non-website stylesheet, as in the screenshot you showed, the
table looks somewhat crammed and visually unappealing; but the website
stylesheet looks pleasing enough.  Screenshot attached.)

> I have only patched GIN to give an idea of the shape of the
> tables.

I suppose a commit would change all the index AMs where we document this
kind of thing.

--
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

gin-opclasses.png (58K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Document "59.2. Built-in Operator Classes" have a clerical error?

Tom Lane-2
Alvaro Herrera <[hidden email]> writes:
> On 2020-Aug-25, Michael Paquier wrote:
>> FWIW, the layout I was thinking about is something like the patch
>> attached.

> This looks to me enough of an improvement that I +1 it, and yes this is
> what I was imagining also.

I think it's a good idea too.

> (With the non-website stylesheet, as in the screenshot you showed, the
> table looks somewhat crammed and visually unappealing; but the website
> stylesheet looks pleasing enough.  Screenshot attached.)

I wonder if it would look better if we suppress the horizontal rules
between the operator names within a cell.  IIRC, it's possible to do
that, though the exact incantation isn't coming to mind right now.

> I suppose a commit would change all the index AMs where we document this
> kind of thing.

Yeah, we should make all these sorts of tables consistent.

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: Document "59.2. Built-in Operator Classes" have a clerical error?

Michael Paquier-2
On Tue, Aug 25, 2020 at 06:17:28PM -0400, Tom Lane wrote:
> Alvaro Herrera <[hidden email]> writes:
>> (With the non-website stylesheet, as in the screenshot you showed, the
>> table looks somewhat crammed and visually unappealing; but the website
>> stylesheet looks pleasing enough.  Screenshot attached.)

Right, STYLE=website.

> I wonder if it would look better if we suppress the horizontal rules
> between the operator names within a cell.  IIRC, it's possible to do
> that, though the exact incantation isn't coming to mind right now.

I would imagine that rowsep=0 for a given <entry> can do that:
https://tdg.docbook.org/tdg/4.5/entry.html
However, it does not make a difference if I use the default style or
the website style.  I may be missing something with the stylesheet
though?

>> I suppose a commit would change all the index AMs where we document this
>> kind of thing.
>
> Yeah, we should make all these sorts of tables consistent.

Of course, just wanted to make sure that we agree on the layer before
spending more time on that as shaping all the tables correctly is a
lot of work.  And GIN is the smallest one.

One thing to note about the BRIN table is that we have never
documented from the start any of the operators working across multiple
types.  For example, we only have 5 operators listed for each one of
int2/int4/int8, timestamptz/timestamp or real/float, but these are
listed with incorrect names and they miss a bunch of other operators
able to handle combinations of (int2,int4), etc.  So I have fixed the
table so as all the operators are listed, grouping together ints,
timestamps and reals, as \dAo says.  Now there could be also an
argument to stick with simplicity and list only the operators of one
single type, reducing the size of the table for BRIN.  But that would
be incorrect with the catalogs, and the attached classifies all
operators in alphabetical order.  (Switching to the original list is
not complicated as that would be just removing code from the attached,
so I took the complicated path for now.)
--
Michael

doc-builtin-ops.patch (55K) Download Attachment
signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Document "59.2. Built-in Operator Classes" have a clerical error?

Alvaro Herrera-9
On 2020-Aug-26, Michael Paquier wrote:

> On Tue, Aug 25, 2020 at 06:17:28PM -0400, Tom Lane wrote:

> > I wonder if it would look better if we suppress the horizontal rules
> > between the operator names within a cell.  IIRC, it's possible to do
> > that, though the exact incantation isn't coming to mind right now.
>
> I would imagine that rowsep=0 for a given <entry> can do that:
> https://tdg.docbook.org/tdg/4.5/entry.html
> However, it does not make a difference if I use the default style or
> the website style.  I may be missing something with the stylesheet
> though?

I have no idea there.  Maybe Jon Katz (CCed) could help you to find an
answer to that question.

> >> I suppose a commit would change all the index AMs where we document this
> >> kind of thing.
> >
> > Yeah, we should make all these sorts of tables consistent.
>
> Of course, just wanted to make sure that we agree on the layer before
> spending more time on that as shaping all the tables correctly is a
> lot of work.  And GIN is the smallest one.

Thanks for doing the legwork!

> One thing to note about the BRIN table is that we have never
> documented from the start any of the operators working across multiple
> types.  For example, we only have 5 operators listed for each one of
> int2/int4/int8, timestamptz/timestamp or real/float, but these are
> listed with incorrect names and they miss a bunch of other operators
> able to handle combinations of (int2,int4), etc.  So I have fixed the
> table so as all the operators are listed, grouping together ints,
> timestamps and reals, as \dAo says.

Well, there is a small problem here ... which is that I misled you with
\dAo ... because that one lists the operators for the opfamilies, not
for the opclasses.  And you can't use the opfamily name in CREATE INDEX:

55432 14devel 30811=# create index on t using brin (a integer_minmax_ops);
ERROR:  operator class "integer_minmax_ops" does not exist for access method "brin"

You have to use the opclass name:

55432 14devel 30811=# create index on t using brin (a int4_minmax_ops);
CREATE INDEX

Compare \dAf to \dAc.  The latter shows what opclasses you can use for
each datatype, while the former shows what types can be used with an
index using the opfamily that the opclass belongs into.

As I understand it, the cross-type operators are "loose" in the opfamily
and they don't belong to any opclass.  So what we could do is list the
opclasses within each opfamily, and then list all the loose operators.
Something like (fixed width font):

Operator family      Operator class     Operator
------------------------------------------------------------
integer_minmax_ops   int4_minmax_ops    =(integer,integer)
                                        <(integer,integer)
                                        >(integer,integer)
                                        >=(integer,integer)
                                        <=(integer,integer)
                     ---------------------------------------
                     int2_minmax_ops    =(smallint,smallint)
                                        etc
                     ----------------------------------------
                     ... other opclasses ...
                     ----------------------------------------
                                        =(smallint,integer)
                                        <(smallint,integer)
                                        >(smallint,integer)
                                        ... rest of operators ...
-------------------------------------------------------------

--
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


12