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

Started by osdbaover 5 years ago28 messagesdocs
Jump to latest
#1osdba
mailtch@163.com

hi all:

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

"range_opsany range type&& &> &< >> << <@ -|- = @> @>", exist double "@>",

Should be "<@ @>" ?

#2Michael Paquier
michael@paquier.xyz
In reply to: osdba (#1)
Re: Document "59.2. Built-in Operator Classes" have a clerical error?

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

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#2)
Re: Document "59.2. Built-in Operator Classes" have a clerical error?

Michael Paquier <michael@paquier.xyz> 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

#4Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#3)
Re: Document "59.2. Built-in Operator Classes" have a clerical error?

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

#5Bruce Momjian
bruce@momjian.us
In reply to: Michael Paquier (#2)
Re: Document "59.2. Built-in Operator Classes" have a clerical error?

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 <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

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

Attachments:

operator.difftext/x-diff; charset=us-asciiDownload+2-5
#6Michael Paquier
michael@paquier.xyz
In reply to: Bruce Momjian (#5)
Re: Document "59.2. Built-in Operator Classes" have a clerical error?

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

#7Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Bruce Momjian (#5)
Re: Document "59.2. Built-in Operator Classes" have a clerical error?

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

#8Michael Paquier
michael@paquier.xyz
In reply to: Alvaro Herrera (#7)
Re: Document "59.2. Built-in Operator Classes" have a clerical error?

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:
/messages/by-id/20200804055651.GC2091@paquier.xyz
--
Michael

#9Bruce Momjian
bruce@momjian.us
In reply to: Michael Paquier (#8)
Re: Document "59.2. Built-in Operator Classes" have a clerical error?

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:
/messages/by-id/20200804055651.GC2091@paquier.xyz

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

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

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

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#9)
Re: Document "59.2. Built-in Operator Classes" have a clerical error?

Bruce Momjian <bruce@momjian.us> 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

#11Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Tom Lane (#10)
Re: Document "59.2. Built-in Operator Classes" have a clerical error?

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

#12Bruce Momjian
bruce@momjian.us
In reply to: Alvaro Herrera (#11)
Re: Document "59.2. Built-in Operator Classes" have a clerical error?

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 <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

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

#13Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Bruce Momjian (#12)
Re: Document "59.2. Built-in Operator Classes" have a clerical error?

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

#14Bruce Momjian
bruce@momjian.us
In reply to: Alvaro Herrera (#13)
Re: Document "59.2. Built-in Operator Classes" have a clerical error?

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 <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

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

#15Michael Paquier
michael@paquier.xyz
In reply to: Bruce Momjian (#14)
Re: Document "59.2. Built-in Operator Classes" have a clerical error?

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

Attachments:

gin_ops_doc.pngimage/pngDownload+0-1
doc-gin-opr-table.patchtext/x-diff; charset=us-asciiDownload+45-31
#16Bruce Momjian
bruce@momjian.us
In reply to: Michael Paquier (#15)
Re: Document "59.2. Built-in Operator Classes" have a clerical error?

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 <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

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

#17Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Michael Paquier (#15)
Re: Document "59.2. Built-in Operator Classes" have a clerical error?

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

Attachments:

gin-opclasses.pngimage/pngDownload
#18Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#17)
Re: Document "59.2. Built-in Operator Classes" have a clerical error?

Alvaro Herrera <alvherre@2ndquadrant.com> 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

#19Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#18)
Re: Document "59.2. Built-in Operator Classes" have a clerical error?

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

Alvaro Herrera <alvherre@2ndquadrant.com> 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

Attachments:

doc-builtin-ops.patchtext/x-diff; charset=us-asciiDownload+540-555
#20Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Michael Paquier (#19)
Re: Document "59.2. Built-in Operator Classes" have a clerical error?

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

#21Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#20)
#22Bruce Momjian
bruce@momjian.us
In reply to: Alvaro Herrera (#20)
#23Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Bruce Momjian (#22)
#24Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#23)
#25Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#24)
#26Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Michael Paquier (#25)
#27Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#26)
#28Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#27)