Обсуждение: Re: Document "59.2. Built-in Operator Classes" have a clerical error?

Поиск
Список
Период
Сортировка

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

От
Alvaro Herrera
Дата:
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



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

От
Tom Lane
Дата:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> On 2020-Aug-26, Michael Paquier wrote:
>> 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.

Yeah, I can't make it work as documented either.  Weird --- I wonder if
our stylesheets are messing that up?  It *does* work in PDF output.

Speaking of PDF output, we have a fair amount of work to do to make these
tables render sanely in PDF.  As the patch stands, it reintroduces a bunch
of "exceed the available area" complaints.  Partly that is because, in the
interests of making the old contents fit, I'd given the operator column
less space than the others:

   <colspec colname="col1" colwidth="2*"/>
   <colspec colname="col2" colwidth="2*"/>
   <colspec colname="col3" colwidth="1*"/>

which is backwards now, of course.  Perhaps you could just drop the
<colspec>s again, or else twiddle their relative widths.  Some other
recommendations I'd make are:

* Don't use the carpal-tunnel-inducing SQL standard datatype names,
but just "timestamp", "timestamptz", etc.

* Put a space after the comma in each operator description; this will
look better and it will cue FOP that that's a good place for a line
break.

* I'm inclined to think that spaces before the operators' left parens
would improve readabilty, too.


> 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)

Hm, do we care quite that much about explaining this difference?
But you're right that we hardly need the "data type" column
per se when the operator column is repeating the same info.

> Thanks for doing the legwork!

Indeed.

            regards, tom lane



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

От
Bruce Momjian
Дата:
On Wed, Aug 26, 2020 at 05:58:01PM -0400, Alvaro Herrera wrote:
> 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):

Stupid question, but do we think the average Postgres user can
understand this issue.  I am having trouble myself.

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

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




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

От
Alvaro Herrera
Дата:
On 2020-Aug-26, Bruce Momjian wrote:

> On Wed, Aug 26, 2020 at 05:58:01PM -0400, Alvaro Herrera wrote:
> > 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):
>
> Stupid question, but do we think the average Postgres user can
> understand this issue.  I am having trouble myself.

The only reason I think it's worth pointing out, is that the opclass
name is something you can use in CREATE INDEX, while the opfamily name
cannot be used there.  The original tables can be used for that purpose,
but the patched tables cannot.

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



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

От
Tom Lane
Дата:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> On 2020-Aug-26, Bruce Momjian wrote:
>> Stupid question, but do we think the average Postgres user can
>> understand this issue.  I am having trouble myself.

> The only reason I think it's worth pointing out, is that the opclass
> name is something you can use in CREATE INDEX, while the opfamily name
> cannot be used there.  The original tables can be used for that purpose,
> but the patched tables cannot.

With one eye on the PDF width issue, I propose that we not draw
the distinction, but just list all the relevant operators for each
opclass (its native ones, plus the applicable "loose" operators).
Then we only need two columns, opclass and operators.

            regards, tom lane