Horton

Thread

Commits

  1. Improvements to psql \dAo and \dAp commands

  1. output columns of \dAo and \dAp

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2020-06-06T17:15:40Z

    I'm somewhat confused by the selection and order of the output columns
produced by the new psql commands \dAo and \dAp (access method operators
and functions, respectively).  Currently, you get

\dAo

 AM  | Operator family |       Operator
-----+-----------------+----------------------
 gin | jsonb_path_ops  | @> (jsonb, jsonb)
...

\dAo+

\dAo+
                                 List of operators of operator families
  AM   | Operator family |                Operator                 | Strategy | Purpose | Sort opfamily
-------+-----------------+-----------------------------------------+----------+---------+---------------
 btree | float_ops       | < (double precision, double precision)  |        1 | search  |
...

\dAp
                        List of support functions of operator families
  AM   | Operator family |  Left arg type   |  Right arg type  | Number |      Function
-------+-----------------+------------------+------------------+--------+---------------------
 btree | float_ops       | double precision | double precision |      1 | btfloat8cmp
...

First, why isn't the strategy number included in the \dAo?  It's part
of the primary key of pg_amop, and it's essential for interpreting the
meaning of the output.

Then there are gratuitous differences in the presentation of \dAo and \dAp.
Why does \dAo show the operator with signature and omit the left arg/right arg
columns, but \dAp shows it the other way around?

I'm also wondering whether this is fully correct.  Would it be possible for the
argument types of the operator/function to differ from the left arg/right arg
types?  (Perhaps binary compatible?)

Either way some more consistency would be welcome.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  2. Re: output columns of \dAo and \dAp

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-06-06T21:34:08Z

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> I'm also wondering whether this is fully correct.  Would it be possible for the
> argument types of the operator/function to differ from the left arg/right arg
> types?  (Perhaps binary compatible?)

pg_amop.h specifies that

 * The primary key for this table is <amopfamily, amoplefttype, amoprighttype,
 * amopstrategy>.  amoplefttype and amoprighttype are just copies of the
 * operator's oprleft/oprright, ie its declared input data types.

Perhaps it'd be a good idea for opr_sanity.sql to verify that, since
it'd be an easy thing to mess up in handmade pg_amop entries.  But
at least for the foreseeable future, there's no reason for \dAo to show
amoplefttype/amoprighttype separately.

I agree that \dAo ought to be showing amopstrategy; moreover that ought
to be much higher in the sort key than it is.  Also, if we're not going
to show amoppurpose, then the view probably ought to hide non-search
operators altogether.  It is REALLY misleading to not distinguish search
and ordering operators.
 
The situation is different for pg_amproc: if you look for discrepancies
you will find plenty, since in many cases a support function's signature
has little to do with what types it is registered under.  Perhaps it'd be
worthwhile for \dAp to show the functions as regprocedure in addition to
showing amproclefttype/amprocrighttype explicitly.  In any case, I think
it's rather misleading for \dAp to label amproclefttype/amprocrighttype as
"Left arg type" and "Right arg type", because for almost everything except
btree/hash, that's not what the support function's arguments actually are.
Perhaps names along the lines of "Registered left type" and "Registered
right type" would put readers in a better frame of mind to understand
the entries.

			regards, tom lane

  3. Re: output columns of \dAo and \dAp

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2020-07-07T21:02:02Z

    Sergey, Nikita, Alexander, if you can please see this thread and propose
a solution, that'd be very welcome.


On 2020-Jun-06, Tom Lane wrote:

> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> > I'm also wondering whether this is fully correct.  Would it be possible for the
> > argument types of the operator/function to differ from the left arg/right arg
> > types?  (Perhaps binary compatible?)
> 
> pg_amop.h specifies that
> 
>  * The primary key for this table is <amopfamily, amoplefttype, amoprighttype,
>  * amopstrategy>.  amoplefttype and amoprighttype are just copies of the
>  * operator's oprleft/oprright, ie its declared input data types.
> 
> Perhaps it'd be a good idea for opr_sanity.sql to verify that, since
> it'd be an easy thing to mess up in handmade pg_amop entries.  But
> at least for the foreseeable future, there's no reason for \dAo to show
> amoplefttype/amoprighttype separately.
> 
> I agree that \dAo ought to be showing amopstrategy; moreover that ought
> to be much higher in the sort key than it is.  Also, if we're not going
> to show amoppurpose, then the view probably ought to hide non-search
> operators altogether.  It is REALLY misleading to not distinguish search
> and ordering operators.
>  
> The situation is different for pg_amproc: if you look for discrepancies
> you will find plenty, since in many cases a support function's signature
> has little to do with what types it is registered under.  Perhaps it'd be
> worthwhile for \dAp to show the functions as regprocedure in addition to
> showing amproclefttype/amprocrighttype explicitly.  In any case, I think
> it's rather misleading for \dAp to label amproclefttype/amprocrighttype as
> "Left arg type" and "Right arg type", because for almost everything except
> btree/hash, that's not what the support function's arguments actually are.
> Perhaps names along the lines of "Registered left type" and "Registered
> right type" would put readers in a better frame of mind to understand
> the entries.
> 
> 			regards, tom lane
> 
> 


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

  4. Re: output columns of \dAo and \dAp

    Alexander Korotkov <aekorotkov@gmail.com> — 2020-07-07T22:09:50Z

    On Sun, Jun 7, 2020 at 12:34 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> > I'm also wondering whether this is fully correct.  Would it be possible for the
> > argument types of the operator/function to differ from the left arg/right arg
> > types?  (Perhaps binary compatible?)
>
> pg_amop.h specifies that
>
>  * The primary key for this table is <amopfamily, amoplefttype, amoprighttype,
>  * amopstrategy>.  amoplefttype and amoprighttype are just copies of the
>  * operator's oprleft/oprright, ie its declared input data types.
>
> Perhaps it'd be a good idea for opr_sanity.sql to verify that, since
> it'd be an easy thing to mess up in handmade pg_amop entries.  But
> at least for the foreseeable future, there's no reason for \dAo to show
> amoplefttype/amoprighttype separately.

+1 for checking consistency of amoplefttype/amoprighttype in opr_sanity.sql

> I agree that \dAo ought to be showing amopstrategy;

I agree that the strategy and purpose of an operator is valuable
information.  And we probably shouldn't hide it in \dAo. If we do so,
then \dAo and \dAo+ differ by only "sort opfamily" column.  Is it
worth keeping the \dAo+ command for single-column difference?

> moreover that ought
> to be much higher in the sort key than it is.

Do you mean we should sort by strategy number and only then by
arg types?  Current output shows operators grouped by opclasses,
after that cross-opclass operators are shown.  This order seems to me
more worthwhile than seeing all the variations of the same strategy
together.

> Also, if we're not going
> to show amoppurpose, then the view probably ought to hide non-search
> operators altogether.  It is REALLY misleading to not distinguish search
> and ordering operators.

+1

> The situation is different for pg_amproc: if you look for discrepancies
> you will find plenty, since in many cases a support function's signature
> has little to do with what types it is registered under.  Perhaps it'd be
> worthwhile for \dAp to show the functions as regprocedure in addition to
> showing amproclefttype/amprocrighttype explicitly.  In any case, I think
> it's rather misleading for \dAp to label amproclefttype/amprocrighttype as
> "Left arg type" and "Right arg type", because for almost everything except
> btree/hash, that's not what the support function's arguments actually are.
> Perhaps names along the lines of "Registered left type" and "Registered
> right type" would put readers in a better frame of mind to understand
> the entries.

+1 for rename "Left arg type"/"Right arg type" to "Registered left
type"/"Registered right type".

------
Regards,
Alexander Korotkov

  5. Re: output columns of \dAo and \dAp

    Jonathan S. Katz <jkatz@postgresql.org> — 2020-07-09T19:03:11Z

    On 7/7/20 6:09 PM, Alexander Korotkov wrote:
> On Sun, Jun 7, 2020 at 12:34 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
>>> I'm also wondering whether this is fully correct.  Would it be possible for the
>>> argument types of the operator/function to differ from the left arg/right arg
>>> types?  (Perhaps binary compatible?)
>>
>> pg_amop.h specifies that
>>
>>  * The primary key for this table is <amopfamily, amoplefttype, amoprighttype,
>>  * amopstrategy>.  amoplefttype and amoprighttype are just copies of the
>>  * operator's oprleft/oprright, ie its declared input data types.
>>
>> Perhaps it'd be a good idea for opr_sanity.sql to verify that, since
>> it'd be an easy thing to mess up in handmade pg_amop entries.  But
>> at least for the foreseeable future, there's no reason for \dAo to show
>> amoplefttype/amoprighttype separately.
> 
> +1 for checking consistency of amoplefttype/amoprighttype in opr_sanity.sql
> 
>> I agree that \dAo ought to be showing amopstrategy;
> 
> I agree that the strategy and purpose of an operator is valuable
> information.  And we probably shouldn't hide it in \dAo. If we do so,
> then \dAo and \dAo+ differ by only "sort opfamily" column.  Is it
> worth keeping the \dAo+ command for single-column difference?
> 
>> moreover that ought
>> to be much higher in the sort key than it is.
> 
> Do you mean we should sort by strategy number and only then by
> arg types?  Current output shows operators grouped by opclasses,
> after that cross-opclass operators are shown.  This order seems to me
> more worthwhile than seeing all the variations of the same strategy
> together.
> 
>> Also, if we're not going
>> to show amoppurpose, then the view probably ought to hide non-search
>> operators altogether.  It is REALLY misleading to not distinguish search
>> and ordering operators.
> 
> +1
> 
>> The situation is different for pg_amproc: if you look for discrepancies
>> you will find plenty, since in many cases a support function's signature
>> has little to do with what types it is registered under.  Perhaps it'd be
>> worthwhile for \dAp to show the functions as regprocedure in addition to
>> showing amproclefttype/amprocrighttype explicitly.  In any case, I think
>> it's rather misleading for \dAp to label amproclefttype/amprocrighttype as
>> "Left arg type" and "Right arg type", because for almost everything except
>> btree/hash, that's not what the support function's arguments actually are.
>> Perhaps names along the lines of "Registered left type" and "Registered
>> right type" would put readers in a better frame of mind to understand
>> the entries.
> 
> +1 for rename "Left arg type"/"Right arg type" to "Registered left
> type"/"Registered right type".

From the RMT perspective, if there is an agreed upon approach (which it
sounds like from the above) can someone please commit to working on
resolving this open item?

Thanks!

Jonathan

  6. Re: output columns of \dAo and \dAp

    Alexander Korotkov <aekorotkov@gmail.com> — 2020-07-09T23:24:19Z

    On Thu, Jul 9, 2020 at 10:03 PM Jonathan S. Katz <jkatz@postgresql.org> wrote:
> From the RMT perspective, if there is an agreed upon approach (which it
> sounds like from the above) can someone please commit to working on
> resolving this open item?

I hardly can extract an approach from this thread, because for me the
whole issue is about details :)

But I think we can come to an agreement shortly.  And yes, I commit to
resolve this.

------
Regards,
Alexander Korotkov

  7. Re: output columns of \dAo and \dAp

    Alexander Korotkov <aekorotkov@gmail.com> — 2020-07-11T11:23:33Z

    On Fri, Jul 10, 2020 at 2:24 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> On Thu, Jul 9, 2020 at 10:03 PM Jonathan S. Katz <jkatz@postgresql.org> wrote:
> > From the RMT perspective, if there is an agreed upon approach (which it
> > sounds like from the above) can someone please commit to working on
> > resolving this open item?
>
> I hardly can extract an approach from this thread, because for me the
> whole issue is about details :)
>
> But I think we can come to an agreement shortly.  And yes, I commit to
> resolve this.

The proposed patch is attached.  This patch is fixes two points:
 * Adds strategy number and purpose to output of \dAo
 * Renames "Left/right arg type" columns of \dAp to "Registered left/right type"

I'm not yet convinced we should change the sort key for \dAo.

Any thoughts?

------
Regards,
Alexander Korotkov

  8. Re: output columns of \dAo and \dAp

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-07-11T19:59:12Z

    Alexander Korotkov <aekorotkov@gmail.com> writes:
> The proposed patch is attached.  This patch is fixes two points:
>  * Adds strategy number and purpose to output of \dAo
>  * Renames "Left/right arg type" columns of \dAp to "Registered left/right type"

I think that \dAp should additionally be changed to print the
function via "oid::regprocedure", not just proname.  A possible
compromise, if you think that's too wordy, is to do it that
way for "\dAp+" while printing plain proname for "\dAp".

BTW, isn't this:

                      "  format ('%%s (%%s, %%s)',\n"
                      "    CASE\n"
                      "      WHEN pg_catalog.pg_operator_is_visible(op.oid) \n"
                      "      THEN op.oprname::pg_catalog.text \n"
                      "      ELSE o.amopopr::pg_catalog.regoper::pg_catalog.text \n"
                      "    END,\n"
                      "    pg_catalog.format_type(o.amoplefttype, NULL),\n"
                      "    pg_catalog.format_type(o.amoprighttype, NULL)\n"
                      "  ) AS \"%s\"\n,"

just an extremely painful way to duplicate the results of regoperator?
(You could likely remove the joins to pg_proc and pg_operator altogether
if you relied on regprocedure and regoperator casts.)

> I'm not yet convinced we should change the sort key for \dAo.

After playing with this more, I'm less worried about that than
I was.  I think I was concerned that the operator name would
sort ahead of amopstrategy, but now I see that the op name isn't
part of the sort key at all.

BTW, these queries seem inadequately schema-qualified, notably
the format() calls.

			regards, tom lane

  9. Re: output columns of \dAo and \dAp

    Alexander Korotkov <aekorotkov@gmail.com> — 2020-07-13T12:44:45Z

    On Sat, Jul 11, 2020 at 10:59 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Alexander Korotkov <aekorotkov@gmail.com> writes:
> > The proposed patch is attached.  This patch is fixes two points:
> >  * Adds strategy number and purpose to output of \dAo
> >  * Renames "Left/right arg type" columns of \dAp to "Registered left/right type"
>
> I think that \dAp should additionally be changed to print the
> function via "oid::regprocedure", not just proname.  A possible
> compromise, if you think that's too wordy, is to do it that
> way for "\dAp+" while printing plain proname for "\dAp".

Good compromise.  Done as you proposed.

> BTW, isn't this:
>
>                       "  format ('%%s (%%s, %%s)',\n"
>                       "    CASE\n"
>                       "      WHEN pg_catalog.pg_operator_is_visible(op.oid) \n"
>                       "      THEN op.oprname::pg_catalog.text \n"
>                       "      ELSE o.amopopr::pg_catalog.regoper::pg_catalog.text \n"
>                       "    END,\n"
>                       "    pg_catalog.format_type(o.amoplefttype, NULL),\n"
>                       "    pg_catalog.format_type(o.amoprighttype, NULL)\n"
>                       "  ) AS \"%s\"\n,"
>
> just an extremely painful way to duplicate the results of regoperator?
> (You could likely remove the joins to pg_proc and pg_operator altogether
> if you relied on regprocedure and regoperator casts.)

Yeah, this subquery is totally dumb.  Replaced with cast to regoperator.

> > I'm not yet convinced we should change the sort key for \dAo.
>
> After playing with this more, I'm less worried about that than
> I was.  I think I was concerned that the operator name would
> sort ahead of amopstrategy, but now I see that the op name isn't
> part of the sort key at all.

Ok.

> BTW, these queries seem inadequately schema-qualified, notably
> the format() calls.

Thank you for pointing.  I've added schema-qualification to pg_catalog
functions and tables.

------
Regards,
Alexander Korotkov

  10. Re: output columns of \dAo and \dAp

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-07-13T14:37:36Z

    Alexander Korotkov <aekorotkov@gmail.com> writes:
> Good compromise.  Done as you proposed.

I'm OK with this version.

			regards, tom lane

  11. Re: output columns of \dAo and \dAp

    Jonathan S. Katz <jkatz@postgresql.org> — 2020-07-13T16:54:50Z

    On 7/13/20 10:37 AM, Tom Lane wrote:
> Alexander Korotkov <aekorotkov@gmail.com> writes:
>> Good compromise.  Done as you proposed.
> 
> I'm OK with this version.

I saw this was committed and the item was adjusted on the Open Items list.

Thank you!

Jonathan

  12. Re: output columns of \dAo and \dAp

    Alexander Korotkov <aekorotkov@gmail.com> — 2020-07-13T16:57:57Z

    On Mon, Jul 13, 2020 at 7:54 PM Jonathan S. Katz <jkatz@postgresql.org> wrote:
> On 7/13/20 10:37 AM, Tom Lane wrote:
> > Alexander Korotkov <aekorotkov@gmail.com> writes:
> >> Good compromise.  Done as you proposed.
> >
> > I'm OK with this version.
>
> I saw this was committed and the item was adjusted on the Open Items list.

Thank you!

------
Regards,
Alexander Korotkov