output columns of \dAo and \dAp

Started by Peter Eisentrautover 5 years ago12 messages
#1Peter Eisentraut
peter.eisentraut@2ndquadrant.com

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

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#1)
Re: output columns of \dAo and \dAp

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

#3Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Tom Lane (#2)
Re: output columns of \dAo and \dAp

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

#4Alexander Korotkov
aekorotkov@gmail.com
In reply to: Tom Lane (#2)
Re: output columns of \dAo and \dAp

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

#5Jonathan S. Katz
jkatz@postgresql.org
In reply to: Alexander Korotkov (#4)
Re: output columns of \dAo and \dAp

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

#6Alexander Korotkov
aekorotkov@gmail.com
In reply to: Jonathan S. Katz (#5)
Re: output columns of \dAo and \dAp

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

#7Alexander Korotkov
aekorotkov@gmail.com
In reply to: Alexander Korotkov (#6)
1 attachment(s)
Re: output columns of \dAo and \dAp

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

Attachments:

0001-Improvements-to-psql-dAo-and-dAp-commands.patchapplication/octet-stream; name=0001-Improvements-to-psql-dAo-and-dAp-commands.patchDownload
From 5c7f56a42d9d428b388be07c70407d8ef6dda82e Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <akorotkov@postgresql.org>
Date: Sat, 11 Jul 2020 14:14:49 +0300
Subject: [PATCH] Improvements to psql \dAo and \dAp commands

 * Strategy number and purpose are essential information for opfamily operator.
   So, show those columns in non-verbose output.
 * "Left/right arg type" \dAp column names are confusing, because those type
   don't necessary match to function arguments.  Rename them to "Registered
   left/right type".

Reported-by: Peter Eisentraut, Tom Lane
Discussion: https://postgr.es/m/2edc7b27-031f-b2b6-0db2-864241c91cb9%402ndquadrant.com
---
 src/bin/psql/describe.c            | 28 ++++++++++-----------
 src/test/regress/expected/psql.out | 50 +++++++++++++++++++-------------------
 2 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index cd39b913cda..135b2ffe5f5 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -6248,23 +6248,23 @@ listOpFamilyOperators(const char *access_method_pattern,
 					  "    END,\n"
 					  "    pg_catalog.format_type(o.amoplefttype, NULL),\n"
 					  "    pg_catalog.format_type(o.amoprighttype, NULL)\n"
-					  "  ) AS \"%s\"\n",
+					  "  ) AS \"%s\"\n,"
+					  "  o.amopstrategy AS \"%s\",\n"
+					  "  CASE o.amoppurpose\n"
+					  "    WHEN 'o' THEN '%s'\n"
+					  "    WHEN 's' THEN '%s'\n"
+					  "  END AS \"%s\"\n",
 					  gettext_noop("AM"),
 					  gettext_noop("Operator family"),
-					  gettext_noop("Operator"));
+					  gettext_noop("Operator"),
+					  gettext_noop("Strategy"),
+					  gettext_noop("ordering"),
+					  gettext_noop("search"),
+					  gettext_noop("Purpose"));
 
 	if (verbose)
 		appendPQExpBuffer(&buf,
-						  ", o.amopstrategy AS \"%s\",\n"
-						  "  CASE o.amoppurpose\n"
-						  "    WHEN 'o' THEN '%s'\n"
-						  "    WHEN 's' THEN '%s'\n"
-						  "  END AS \"%s\",\n"
-						  "  ofs.opfname AS \"%s\"\n",
-						  gettext_noop("Strategy"),
-						  gettext_noop("ordering"),
-						  gettext_noop("search"),
-						  gettext_noop("Purpose"),
+						  ", ofs.opfname AS \"%s\"\n",
 						  gettext_noop("Sort opfamily"));
 	appendPQExpBuffer(&buf,
 					  "FROM pg_catalog.pg_amop o\n"
@@ -6341,8 +6341,8 @@ listOpFamilyFunctions(const char *access_method_pattern,
 					  "  p.proname AS \"%s\"\n",
 					  gettext_noop("AM"),
 					  gettext_noop("Operator family"),
-					  gettext_noop("Left arg type"),
-					  gettext_noop("Right arg type"),
+					  gettext_noop("Registered left type"),
+					  gettext_noop("Registered right type"),
 					  gettext_noop("Number"),
 					  gettext_noop("Function"));
 
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 7d2d6328fc8..ae4d1e58072 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -4979,36 +4979,36 @@ List of access methods
 (20 rows)
 
 \dAo * pg_catalog.jsonb_path_ops
-    List of operators of operator families
- AM  | Operator family |       Operator       
------+-----------------+----------------------
- gin | jsonb_path_ops  | @> (jsonb, jsonb)
- gin | jsonb_path_ops  | @? (jsonb, jsonpath)
- gin | jsonb_path_ops  | @@ (jsonb, jsonpath)
+              List of operators of operator families
+ AM  | Operator family |       Operator       | Strategy | Purpose 
+-----+-----------------+----------------------+----------+---------
+ gin | jsonb_path_ops  | @> (jsonb, jsonb)    |        7 | search
+ gin | jsonb_path_ops  | @? (jsonb, jsonpath) |       15 | search
+ gin | jsonb_path_ops  | @@ (jsonb, jsonpath) |       16 | search
 (3 rows)
 
 \dAp btree float_ops
-                        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
- btree | float_ops       | double precision | double precision |      2 | btfloat8sortsupport
- btree | float_ops       | double precision | double precision |      3 | in_range
- btree | float_ops       | real             | real             |      1 | btfloat4cmp
- btree | float_ops       | real             | real             |      2 | btfloat4sortsupport
- btree | float_ops       | double precision | real             |      1 | btfloat84cmp
- btree | float_ops       | real             | double precision |      1 | btfloat48cmp
- btree | float_ops       | real             | double precision |      3 | in_range
+                            List of support functions of operator families
+  AM   | Operator family | Registered left type | Registered right type | Number |      Function       
+-------+-----------------+----------------------+-----------------------+--------+---------------------
+ btree | float_ops       | double precision     | double precision      |      1 | btfloat8cmp
+ btree | float_ops       | double precision     | double precision      |      2 | btfloat8sortsupport
+ btree | float_ops       | double precision     | double precision      |      3 | in_range
+ btree | float_ops       | real                 | real                  |      1 | btfloat4cmp
+ btree | float_ops       | real                 | real                  |      2 | btfloat4sortsupport
+ btree | float_ops       | double precision     | real                  |      1 | btfloat84cmp
+ btree | float_ops       | real                 | double precision      |      1 | btfloat48cmp
+ btree | float_ops       | real                 | double precision      |      3 | in_range
 (8 rows)
 
 \dAp * pg_catalog.uuid_ops
-                     List of support functions of operator families
-  AM   | Operator family | Left arg type | Right arg type | Number |      Function      
--------+-----------------+---------------+----------------+--------+--------------------
- btree | uuid_ops        | uuid          | uuid           |      1 | uuid_cmp
- btree | uuid_ops        | uuid          | uuid           |      2 | uuid_sortsupport
- btree | uuid_ops        | uuid          | uuid           |      4 | btequalimage
- hash  | uuid_ops        | uuid          | uuid           |      1 | uuid_hash
- hash  | uuid_ops        | uuid          | uuid           |      2 | uuid_hash_extended
+                            List of support functions of operator families
+  AM   | Operator family | Registered left type | Registered right type | Number |      Function      
+-------+-----------------+----------------------+-----------------------+--------+--------------------
+ btree | uuid_ops        | uuid                 | uuid                  |      1 | uuid_cmp
+ btree | uuid_ops        | uuid                 | uuid                  |      2 | uuid_sortsupport
+ btree | uuid_ops        | uuid                 | uuid                  |      4 | btequalimage
+ hash  | uuid_ops        | uuid                 | uuid                  |      1 | uuid_hash
+ hash  | uuid_ops        | uuid                 | uuid                  |      2 | uuid_hash_extended
 (5 rows)
 
-- 
2.14.3

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alexander Korotkov (#7)
Re: output columns of \dAo and \dAp

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

#9Alexander Korotkov
aekorotkov@gmail.com
In reply to: Tom Lane (#8)
1 attachment(s)
Re: output columns of \dAo and \dAp

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

Attachments:

0001-Improvements-to-psql-dAo-and-dAp-commands-2.patchapplication/octet-stream; name=0001-Improvements-to-psql-dAo-and-dAp-commands-2.patchDownload
From 15c5f53f25212120f6a1619e628d32b8a98e5633 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <akorotkov@postgresql.org>
Date: Sat, 11 Jul 2020 14:14:49 +0300
Subject: [PATCH] Improvements to psql \dAo and \dAp commands

 * Strategy number and purpose are essential information for opfamily operator.
   So, show those columns in non-verbose output.
 * "Left/right arg type" \dAp column names are confusing, because those type
   don't necessary match to function arguments.  Rename them to "Registered
   left/right type".
 * Replace manual assembling of operator/procedure names with casts to
   regoperator/regprocedure.
 * Add schema-qualification for pg_catalog functions and tables.

Reported-by: Peter Eisentraut, Tom Lane
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/2edc7b27-031f-b2b6-0db2-864241c91cb9%402ndquadrant.com
Backpatch-through: 13
---
 src/bin/psql/command.c             |  2 +-
 src/bin/psql/describe.c            | 93 ++++++++++++++++++------------------
 src/bin/psql/describe.h            |  2 +-
 src/test/regress/expected/psql.out | 98 +++++++++++++++++++-------------------
 src/test/regress/sql/psql.sql      |  2 +-
 5 files changed, 98 insertions(+), 99 deletions(-)

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 560eacc7f0c..9902a4a2ba8 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -747,7 +747,7 @@ exec_command_d(PsqlScanState scan_state, bool active_branch, const char *cmd)
 							success = listOpFamilyOperators(pattern, pattern2, show_verbose);
 							break;
 						case 'p':
-							success = listOpFamilyFunctions(pattern, pattern2);
+							success = listOpFamilyFunctions(pattern, pattern2, show_verbose);
 							break;
 						default:
 							status = PSQL_CMD_UNKNOWN;
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index cd39b913cda..3b870c3b17e 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -6067,15 +6067,16 @@ listOperatorClasses(const char *access_method_pattern,
 	printfPQExpBuffer(&buf,
 					  "SELECT DISTINCT"
 					  "  am.amname AS \"%s\",\n"
-					  "  c.opcintype::pg_catalog.regtype AS \"%s\",\n"
-					  "  (CASE WHEN c.opckeytype <> 0 AND c.opckeytype <> c.opcintype\n"
-					  "    THEN c.opckeytype\n"
-					  "    ELSE NULL -- c.opcintype\n"
-					  "  END)::pg_catalog.regtype AS \"%s\",\n"
+					  "  pg_catalog.format_type(c.opcintype, NULL) AS \"%s\",\n"
+					  "  CASE\n"
+					  "    WHEN c.opckeytype <> 0 AND c.opckeytype <> c.opcintype\n"
+					  "    THEN pg_catalog.format_type(c.opckeytype, NULL)\n"
+					  "    ELSE NULL\n"
+					  "  END AS \"%s\",\n"
 					  "  CASE\n"
 					  "    WHEN pg_catalog.pg_opclass_is_visible(c.oid)\n"
-					  "    THEN format('%%I', c.opcname)\n"
-					  "    ELSE format('%%I.%%I', n.nspname, c.opcname)\n"
+					  "    THEN pg_catalog.format('%%I', c.opcname)\n"
+					  "    ELSE pg_catalog.format('%%I.%%I', n.nspname, c.opcname)\n"
 					  "  END AS \"%s\",\n"
 					  "  (CASE WHEN c.opcdefault\n"
 					  "    THEN '%s'\n"
@@ -6092,8 +6093,8 @@ listOperatorClasses(const char *access_method_pattern,
 		appendPQExpBuffer(&buf,
 						  ",\n  CASE\n"
 						  "    WHEN pg_catalog.pg_opfamily_is_visible(of.oid)\n"
-						  "    THEN format('%%I', of.opfname)\n"
-						  "    ELSE format('%%I.%%I', ofn.nspname, of.opfname)\n"
+						  "    THEN pg_catalog.format('%%I', of.opfname)\n"
+						  "    ELSE pg_catalog.format('%%I.%%I', ofn.nspname, of.opfname)\n"
 						  "  END AS \"%s\",\n"
 						  " pg_catalog.pg_get_userbyid(c.opcowner) AS \"%s\"\n",
 						  gettext_noop("Operator family"),
@@ -6157,12 +6158,12 @@ listOperatorFamilies(const char *access_method_pattern,
 					  "  am.amname AS \"%s\",\n"
 					  "  CASE\n"
 					  "    WHEN pg_catalog.pg_opfamily_is_visible(f.oid)\n"
-					  "    THEN format('%%I', f.opfname)\n"
-					  "    ELSE format('%%I.%%I', n.nspname, f.opfname)\n"
+					  "    THEN pg_catalog.format('%%I', f.opfname)\n"
+					  "    ELSE pg_catalog.format('%%I.%%I', n.nspname, f.opfname)\n"
 					  "  END AS \"%s\",\n"
 					  "  (SELECT\n"
-					  "     string_agg(format_type(oc.opcintype, -1), ', ')\n"
-					  "   FROM pg_opclass oc\n"
+					  "     pg_catalog.string_agg(pg_catalog.format_type(oc.opcintype, NULL), ', ')\n"
+					  "   FROM pg_catalog.pg_opclass oc\n"
 					  "   WHERE oc.opcfamily = f.oid) \"%s\"",
 					  gettext_noop("AM"),
 					  gettext_noop("Operator family"),
@@ -6185,8 +6186,8 @@ listOperatorFamilies(const char *access_method_pattern,
 		appendPQExpBuffer(&buf,
 						  "\n  %s EXISTS (\n"
 						  "    SELECT 1\n"
-						  "    FROM pg_type t\n"
-						  "    JOIN pg_opclass oc ON oc.opcintype = t.oid\n"
+						  "    FROM pg_catalog.pg_type t\n"
+						  "    JOIN pg_catalog.pg_opclass oc ON oc.opcintype = t.oid\n"
 						  "    WHERE oc.opcfamily = f.oid",
 						  have_where ? "AND" : "WHERE");
 		processSQLNamePattern(pset.db, &buf, type_pattern, true, false,
@@ -6237,38 +6238,29 @@ listOpFamilyOperators(const char *access_method_pattern,
 					  "  am.amname AS \"%s\",\n"
 					  "  CASE\n"
 					  "    WHEN pg_catalog.pg_opfamily_is_visible(of.oid)\n"
-					  "    THEN format('%%I', of.opfname)\n"
-					  "    ELSE format('%%I.%%I', nsf.nspname, of.opfname)\n"
+					  "    THEN pg_catalog.format('%%I', of.opfname)\n"
+					  "    ELSE pg_catalog.format('%%I.%%I', nsf.nspname, of.opfname)\n"
 					  "  END AS \"%s\",\n"
-					  "  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",
+					  "  o.amopopr::pg_catalog.regoperator AS \"%s\"\n,"
+					  "  o.amopstrategy AS \"%s\",\n"
+					  "  CASE o.amoppurpose\n"
+					  "    WHEN 'o' THEN '%s'\n"
+					  "    WHEN 's' THEN '%s'\n"
+					  "  END AS \"%s\"\n",
 					  gettext_noop("AM"),
 					  gettext_noop("Operator family"),
-					  gettext_noop("Operator"));
+					  gettext_noop("Operator"),
+					  gettext_noop("Strategy"),
+					  gettext_noop("ordering"),
+					  gettext_noop("search"),
+					  gettext_noop("Purpose"));
 
 	if (verbose)
 		appendPQExpBuffer(&buf,
-						  ", o.amopstrategy AS \"%s\",\n"
-						  "  CASE o.amoppurpose\n"
-						  "    WHEN 'o' THEN '%s'\n"
-						  "    WHEN 's' THEN '%s'\n"
-						  "  END AS \"%s\",\n"
-						  "  ofs.opfname AS \"%s\"\n",
-						  gettext_noop("Strategy"),
-						  gettext_noop("ordering"),
-						  gettext_noop("search"),
-						  gettext_noop("Purpose"),
+						  ", ofs.opfname AS \"%s\"\n",
 						  gettext_noop("Sort opfamily"));
 	appendPQExpBuffer(&buf,
 					  "FROM pg_catalog.pg_amop o\n"
-					  "  LEFT JOIN pg_catalog.pg_operator op ON op.oid = o.amopopr\n"
 					  "  LEFT JOIN pg_catalog.pg_opfamily of ON of.oid = o.amopfamily\n"
 					  "  LEFT JOIN pg_catalog.pg_am am ON am.oid = of.opfmethod AND am.oid = o.amopmethod\n"
 					  "  LEFT JOIN pg_catalog.pg_namespace nsf ON of.opfnamespace = nsf.oid\n");
@@ -6317,7 +6309,7 @@ listOpFamilyOperators(const char *access_method_pattern,
  */
 bool
 listOpFamilyFunctions(const char *access_method_pattern,
-					  const char *family_pattern)
+					  const char *family_pattern, bool verbose)
 {
 	PQExpBufferData buf;
 	PGresult   *res;
@@ -6332,19 +6324,26 @@ listOpFamilyFunctions(const char *access_method_pattern,
 					  "  am.amname AS \"%s\",\n"
 					  "  CASE\n"
 					  "    WHEN pg_catalog.pg_opfamily_is_visible(of.oid)\n"
-					  "    THEN format('%%I', of.opfname)\n"
-					  "    ELSE format('%%I.%%I', ns.nspname, of.opfname)\n"
+					  "    THEN pg_catalog.format('%%I', of.opfname)\n"
+					  "    ELSE pg_catalog.format('%%I.%%I', ns.nspname, of.opfname)\n"
 					  "  END AS \"%s\",\n"
 					  "  pg_catalog.format_type(ap.amproclefttype, NULL) AS \"%s\",\n"
 					  "  pg_catalog.format_type(ap.amprocrighttype, NULL) AS \"%s\",\n"
-					  "  ap.amprocnum AS \"%s\"\n,"
-					  "  p.proname AS \"%s\"\n",
+					  "  ap.amprocnum AS \"%s\"\n",
 					  gettext_noop("AM"),
 					  gettext_noop("Operator family"),
-					  gettext_noop("Left arg type"),
-					  gettext_noop("Right arg type"),
-					  gettext_noop("Number"),
-					  gettext_noop("Function"));
+					  gettext_noop("Registered left type"),
+					  gettext_noop("Registered right type"),
+					  gettext_noop("Number"));
+
+	if (!verbose)
+		appendPQExpBuffer(&buf,
+						  ", p.proname AS \"%s\"\n",
+						  gettext_noop("Function"));
+	else
+		appendPQExpBuffer(&buf,
+						  ", ap.amproc::pg_catalog.regprocedure AS \"%s\"\n",
+						  gettext_noop("Function"));
 
 	appendPQExpBuffer(&buf,
 					  "FROM pg_catalog.pg_amproc ap\n"
diff --git a/src/bin/psql/describe.h b/src/bin/psql/describe.h
index 4297f7fdfdf..f0e3ec957c0 100644
--- a/src/bin/psql/describe.h
+++ b/src/bin/psql/describe.h
@@ -130,7 +130,7 @@ extern bool listOpFamilyOperators(const char *accessMethod_pattern,
 
 /* \dAp */
 extern bool listOpFamilyFunctions(const char *access_method_pattern,
-								  const char *family_pattern);
+								  const char *family_pattern, bool verbose);
 
 
 #endif							/* DESCRIBE_H */
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 7d2d6328fc8..555d464f918 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -4953,62 +4953,62 @@ List of access methods
 (1 row)
 
 \dAo+ btree float_ops
-                                 List of operators of operator families
-  AM   | Operator family |                Operator                 | Strategy | Purpose | Sort opfamily 
--------+-----------------+-----------------------------------------+----------+---------+---------------
- btree | float_ops       | < (double precision, double precision)  |        1 | search  | 
- btree | float_ops       | <= (double precision, double precision) |        2 | search  | 
- btree | float_ops       | = (double precision, double precision)  |        3 | search  | 
- btree | float_ops       | >= (double precision, double precision) |        4 | search  | 
- btree | float_ops       | > (double precision, double precision)  |        5 | search  | 
- btree | float_ops       | < (real, real)                          |        1 | search  | 
- btree | float_ops       | <= (real, real)                         |        2 | search  | 
- btree | float_ops       | = (real, real)                          |        3 | search  | 
- btree | float_ops       | >= (real, real)                         |        4 | search  | 
- btree | float_ops       | > (real, real)                          |        5 | search  | 
- btree | float_ops       | < (double precision, real)              |        1 | search  | 
- btree | float_ops       | <= (double precision, real)             |        2 | search  | 
- btree | float_ops       | = (double precision, real)              |        3 | search  | 
- btree | float_ops       | >= (double precision, real)             |        4 | search  | 
- btree | float_ops       | > (double precision, real)              |        5 | search  | 
- btree | float_ops       | < (real, double precision)              |        1 | search  | 
- btree | float_ops       | <= (real, double precision)             |        2 | search  | 
- btree | float_ops       | = (real, double precision)              |        3 | search  | 
- btree | float_ops       | >= (real, double precision)             |        4 | search  | 
- btree | float_ops       | > (real, double precision)              |        5 | search  | 
+                                List of operators of operator families
+  AM   | Operator family |               Operator                | Strategy | Purpose | Sort opfamily 
+-------+-----------------+---------------------------------------+----------+---------+---------------
+ btree | float_ops       | <(double precision,double precision)  |        1 | search  | 
+ btree | float_ops       | <=(double precision,double precision) |        2 | search  | 
+ btree | float_ops       | =(double precision,double precision)  |        3 | search  | 
+ btree | float_ops       | >=(double precision,double precision) |        4 | search  | 
+ btree | float_ops       | >(double precision,double precision)  |        5 | search  | 
+ btree | float_ops       | <(real,real)                          |        1 | search  | 
+ btree | float_ops       | <=(real,real)                         |        2 | search  | 
+ btree | float_ops       | =(real,real)                          |        3 | search  | 
+ btree | float_ops       | >=(real,real)                         |        4 | search  | 
+ btree | float_ops       | >(real,real)                          |        5 | search  | 
+ btree | float_ops       | <(double precision,real)              |        1 | search  | 
+ btree | float_ops       | <=(double precision,real)             |        2 | search  | 
+ btree | float_ops       | =(double precision,real)              |        3 | search  | 
+ btree | float_ops       | >=(double precision,real)             |        4 | search  | 
+ btree | float_ops       | >(double precision,real)              |        5 | search  | 
+ btree | float_ops       | <(real,double precision)              |        1 | search  | 
+ btree | float_ops       | <=(real,double precision)             |        2 | search  | 
+ btree | float_ops       | =(real,double precision)              |        3 | search  | 
+ btree | float_ops       | >=(real,double precision)             |        4 | search  | 
+ btree | float_ops       | >(real,double precision)              |        5 | search  | 
 (20 rows)
 
 \dAo * pg_catalog.jsonb_path_ops
-    List of operators of operator families
- AM  | Operator family |       Operator       
------+-----------------+----------------------
- gin | jsonb_path_ops  | @> (jsonb, jsonb)
- gin | jsonb_path_ops  | @? (jsonb, jsonpath)
- gin | jsonb_path_ops  | @@ (jsonb, jsonpath)
+             List of operators of operator families
+ AM  | Operator family |      Operator      | Strategy | Purpose 
+-----+-----------------+--------------------+----------+---------
+ gin | jsonb_path_ops  | @>(jsonb,jsonb)    |        7 | search
+ gin | jsonb_path_ops  | @?(jsonb,jsonpath) |       15 | search
+ gin | jsonb_path_ops  | @@(jsonb,jsonpath) |       16 | search
 (3 rows)
 
-\dAp btree float_ops
-                        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
- btree | float_ops       | double precision | double precision |      2 | btfloat8sortsupport
- btree | float_ops       | double precision | double precision |      3 | in_range
- btree | float_ops       | real             | real             |      1 | btfloat4cmp
- btree | float_ops       | real             | real             |      2 | btfloat4sortsupport
- btree | float_ops       | double precision | real             |      1 | btfloat84cmp
- btree | float_ops       | real             | double precision |      1 | btfloat48cmp
- btree | float_ops       | real             | double precision |      3 | in_range
+\dAp+ btree float_ops
+                                                         List of support functions of operator families
+  AM   | Operator family | Registered left type | Registered right type | Number |                                   Function                                   
+-------+-----------------+----------------------+-----------------------+--------+------------------------------------------------------------------------------
+ btree | float_ops       | double precision     | double precision      |      1 | btfloat8cmp(double precision,double precision)
+ btree | float_ops       | double precision     | double precision      |      2 | btfloat8sortsupport(internal)
+ btree | float_ops       | double precision     | double precision      |      3 | in_range(double precision,double precision,double precision,boolean,boolean)
+ btree | float_ops       | real                 | real                  |      1 | btfloat4cmp(real,real)
+ btree | float_ops       | real                 | real                  |      2 | btfloat4sortsupport(internal)
+ btree | float_ops       | double precision     | real                  |      1 | btfloat84cmp(double precision,real)
+ btree | float_ops       | real                 | double precision      |      1 | btfloat48cmp(real,double precision)
+ btree | float_ops       | real                 | double precision      |      3 | in_range(real,real,double precision,boolean,boolean)
 (8 rows)
 
 \dAp * pg_catalog.uuid_ops
-                     List of support functions of operator families
-  AM   | Operator family | Left arg type | Right arg type | Number |      Function      
--------+-----------------+---------------+----------------+--------+--------------------
- btree | uuid_ops        | uuid          | uuid           |      1 | uuid_cmp
- btree | uuid_ops        | uuid          | uuid           |      2 | uuid_sortsupport
- btree | uuid_ops        | uuid          | uuid           |      4 | btequalimage
- hash  | uuid_ops        | uuid          | uuid           |      1 | uuid_hash
- hash  | uuid_ops        | uuid          | uuid           |      2 | uuid_hash_extended
+                            List of support functions of operator families
+  AM   | Operator family | Registered left type | Registered right type | Number |      Function      
+-------+-----------------+----------------------+-----------------------+--------+--------------------
+ btree | uuid_ops        | uuid                 | uuid                  |      1 | uuid_cmp
+ btree | uuid_ops        | uuid                 | uuid                  |      2 | uuid_sortsupport
+ btree | uuid_ops        | uuid                 | uuid                  |      4 | btequalimage
+ hash  | uuid_ops        | uuid                 | uuid                  |      1 | uuid_hash
+ hash  | uuid_ops        | uuid                 | uuid                  |      2 | uuid_hash_extended
 (5 rows)
 
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index bd10aec6d67..5a160809807 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -1205,5 +1205,5 @@ drop role regress_partitioning_role;
 \dAf btree int4
 \dAo+ btree float_ops
 \dAo * pg_catalog.jsonb_path_ops
-\dAp btree float_ops
+\dAp+ btree float_ops
 \dAp * pg_catalog.uuid_ops
-- 
2.14.3

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alexander Korotkov (#9)
Re: output columns of \dAo and \dAp

Alexander Korotkov <aekorotkov@gmail.com> writes:

Good compromise. Done as you proposed.

I'm OK with this version.

regards, tom lane

#11Jonathan S. Katz
jkatz@postgresql.org
In reply to: Tom Lane (#10)
Re: output columns of \dAo and \dAp

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

#12Alexander Korotkov
aekorotkov@gmail.com
In reply to: Jonathan S. Katz (#11)
Re: output columns of \dAo and \dAp

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