Display oprcode and its volatility in \do+

Started by Marko Tiikkajaabout 12 years ago9 messages

marko@joh.to

about 12 years ago

1 attachment(s)

Hi,

Here's a patch for $SUBJECT, displaying information which I find quite
tedious to locate using alternative methods. Hopefully someone else
does, too. Or doesn't. Not sure.

Regards,
Marko Tiikkaja

Attachments:

doplus.patchtext/plain; charset=UTF-8; name=doplus.patch; x-mac-creator=0; x-mac-type=0Download

*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
***************
*** 410,416 **** exec_command(const char *cmd,
  				success = listSchemas(pattern, show_verbose, show_system);
  				break;
  			case 'o':
! 				success = describeOperators(pattern, show_system);
  				break;
  			case 'O':
  				success = listCollations(pattern, show_verbose, show_system);
--- 410,416 ----
  				success = listSchemas(pattern, show_verbose, show_system);
  				break;
  			case 'o':
! 				success = describeOperators(pattern, show_verbose, show_system);
  				break;
  			case 'O':
  				success = listCollations(pattern, show_verbose, show_system);
*** a/src/bin/psql/describe.c
--- b/src/bin/psql/describe.c
***************
*** 579,585 **** describeTypes(const char *pattern, bool verbose, bool showSystem)
  /* \do
   */
  bool
! describeOperators(const char *pattern, bool showSystem)
  {
  	PQExpBufferData buf;
  	PGresult   *res;
--- 579,585 ----
  /* \do
   */
  bool
! describeOperators(const char *pattern, bool verbose, bool showSystem)
  {
  	PQExpBufferData buf;
  	PGresult   *res;
***************
*** 605,615 **** describeOperators(const char *pattern, bool showSystem)
  					  "  o.oprname AS \"%s\",\n"
  					  "  CASE WHEN o.oprkind='l' THEN NULL ELSE pg_catalog.format_type(o.oprleft, NULL) END AS \"%s\",\n"
  					  "  CASE WHEN o.oprkind='r' THEN NULL ELSE pg_catalog.format_type(o.oprright, NULL) END AS \"%s\",\n"
! 				   "  pg_catalog.format_type(o.oprresult, NULL) AS \"%s\",\n"
! 			 "  coalesce(pg_catalog.obj_description(o.oid, 'pg_operator'),\n"
! 	"           pg_catalog.obj_description(o.oprcode, 'pg_proc')) AS \"%s\"\n"
! 					  "FROM pg_catalog.pg_operator o\n"
! 	  "     LEFT JOIN pg_catalog.pg_namespace n ON n.oid = o.oprnamespace\n",
  					  gettext_noop("Schema"),
  					  gettext_noop("Name"),
  					  gettext_noop("Left arg type"),
--- 605,613 ----
  					  "  o.oprname AS \"%s\",\n"
  					  "  CASE WHEN o.oprkind='l' THEN NULL ELSE pg_catalog.format_type(o.oprleft, NULL) END AS \"%s\",\n"
  					  "  CASE WHEN o.oprkind='r' THEN NULL ELSE pg_catalog.format_type(o.oprright, NULL) END AS \"%s\",\n"
! 					  "  pg_catalog.format_type(o.oprresult, NULL) AS \"%s\",\n"
! 					  "  coalesce(pg_catalog.obj_description(o.oid, 'pg_operator'),\n"
! 					  "           pg_catalog.obj_description(o.oprcode, 'pg_proc')) AS \"%s\"\n",
  					  gettext_noop("Schema"),
  					  gettext_noop("Name"),
  					  gettext_noop("Left arg type"),
***************
*** 617,622 **** describeOperators(const char *pattern, bool showSystem)
--- 615,641 ----
  					  gettext_noop("Result type"),
  					  gettext_noop("Description"));
  
+ 	if (verbose)
+ 		appendPQExpBuffer(&buf,
+ 						  ",\n  o.oprcode AS \"%s\",\n"
+ 						  "  CASE\n"
+ 						  "    WHEN p.provolatile = 'i' THEN '%s'\n"
+ 						  "    WHEN p.provolatile = 's' THEN '%s'\n"
+ 						  "    WHEN p.provolatile = 'v' THEN '%s'\n"
+ 						  "  END AS \"%s\"\n",
+ 						  gettext_noop("Function"),
+ 						  gettext_noop("immutable"),
+ 						  gettext_noop("stable"),
+ 						  gettext_noop("volatile"),
+ 						  gettext_noop("Volatility"));
+ 
+ 	appendPQExpBuffer(&buf,
+ 					  "FROM pg_catalog.pg_operator o\n"
+ 					  "     LEFT JOIN pg_catalog.pg_namespace n ON n.oid = o.oprnamespace\n");
+ 	if (verbose)
+ 		appendPQExpBuffer(&buf,
+ 						  "\n    LEFT JOIN pg_catalog.pg_proc p ON p.oid = o.oprcode\n");
+ 
  	if (!showSystem && !pattern)
  		appendPQExpBufferStr(&buf, "WHERE n.nspname <> 'pg_catalog'\n"
  							 "      AND n.nspname <> 'information_schema'\n");
*** a/src/bin/psql/describe.h
--- b/src/bin/psql/describe.h
***************
*** 22,28 **** extern bool describeFunctions(const char *functypes, const char *pattern, bool v
  extern bool describeTypes(const char *pattern, bool verbose, bool showSystem);
  
  /* \do */
! extern bool describeOperators(const char *pattern, bool showSystem);
  
  /* \du, \dg */
  extern bool describeRoles(const char *pattern, bool verbose);
--- 22,28 ----
  extern bool describeTypes(const char *pattern, bool verbose, bool showSystem);
  
  /* \do */
! extern bool describeOperators(const char *pattern, bool verbose, bool showSystem);
  
  /* \du, \dg */
  extern bool describeRoles(const char *pattern, bool verbose);

Rushabh Lathia

rushabh.lathia@gmail.com

almost 12 years ago

In reply to: Marko Tiikkaja (#1)

Re: Display oprcode and its volatility in \do+

Hi,

I have reviewed you patch.

-- Patch got applied cleanly (using patch -p1)
-- Make & Make install works fine
-- make check looks good

I done code-walk and it looks good. Also did some manual testing and haven't
found any issue with the implementation.

Even I personally felt the Function and Volatility is nice to have info
into \do+.

Marking it as ready for committer.

Regards,
Rushabh Lathia
www.EnterpriseDB.com

On Fri, Jan 10, 2014 at 7:12 AM, Marko Tiikkaja <marko@joh.to> wrote:

Hi,

Here's a patch for $SUBJECT, displaying information which I find quite
tedious to locate using alternative methods. Hopefully someone else does,
too. Or doesn't. Not sure.

Regards,
Marko Tiikkaja

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
Rushabh Lathia

Marko Tiikkaja

marko@joh.to

almost 12 years ago

In reply to: Rushabh Lathia (#2)

Re: Display oprcode and its volatility in \do+

On 1/16/14 9:53 AM, Rushabh Lathia wrote:

I have reviewed you patch.

-- Patch got applied cleanly (using patch -p1)
-- Make & Make install works fine
-- make check looks good

I done code-walk and it looks good. Also did some manual testing and haven't
found any issue with the implementation.

Even I personally felt the Function and Volatility is nice to have info
into \do+.

Marking it as ready for committer.

Thanks for reviewing!

Regards,
Marko Tiikkaja

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane

tgl@sss.pgh.pa.us

almost 12 years ago

In reply to: Marko Tiikkaja (#3)

Re: Display oprcode and its volatility in \do+

Marko Tiikkaja <marko@joh.to> writes:

On 1/16/14 9:53 AM, Rushabh Lathia wrote:

Even I personally felt the Function and Volatility is nice to have info
into \do+.

FWIW, I'm on board with the idea of printing the oprcode, but adding
volatility here seems like probably a waste of valuable terminal width.
I think that the vast majority of operators have immutable or at worst
stable underlying functions, so this doesn't seem like the first bit
of information I'd need about the underlying function. And why print
this but not, say, security, owner, source code, or other columns
shown in \df? ISTM the value of this addition is to give you what
you need to go look in \df, not to try to substitute for that.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Marko Tiikkaja

marko@joh.to

almost 12 years ago

In reply to: Tom Lane (#4)

Re: Display oprcode and its volatility in \do+

On 1/16/14 4:22 PM, Tom Lane wrote:

Marko Tiikkaja <marko@joh.to> writes:

On 1/16/14 9:53 AM, Rushabh Lathia wrote:

Even I personally felt the Function and Volatility is nice to have info
into \do+.

FWIW, I'm on board with the idea of printing the oprcode, but adding
volatility here seems like probably a waste of valuable terminal width.
I think that the vast majority of operators have immutable or at worst
stable underlying functions, so this doesn't seem like the first bit
of information I'd need about the underlying function.

Completely unscientifically, 50% of the time I've wanted to know the
oprcode has been because I wanted to know its volatility (exactly
because of the stable oprcodes we have). It seemed like a useful
addition, but I don't feel that strongly about it.

Regards,
Marko Tiikkaja

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane

tgl@sss.pgh.pa.us

almost 12 years ago

In reply to: Marko Tiikkaja (#5)

Re: Display oprcode and its volatility in \do+

Marko Tiikkaja <marko@joh.to> writes:

On 1/16/14 4:22 PM, Tom Lane wrote:

FWIW, I'm on board with the idea of printing the oprcode, but adding
volatility here seems like probably a waste of valuable terminal width.
I think that the vast majority of operators have immutable or at worst
stable underlying functions, so this doesn't seem like the first bit
of information I'd need about the underlying function.

Completely unscientifically, 50% of the time I've wanted to know the
oprcode has been because I wanted to know its volatility (exactly
because of the stable oprcodes we have). It seemed like a useful
addition, but I don't feel that strongly about it.

Hm. Personally, I've lost count of the number of times I've had to
resort to "select ... from pg_operator" because \do lacked an oprcode
column, but I don't remember that many or indeed any were because
I wanted to check the volatility.

Anybody else have an opinion?

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane

tgl@sss.pgh.pa.us

almost 12 years ago

In reply to: Tom Lane (#6)

Re: Display oprcode and its volatility in \do+

I wrote:

Anybody else have an opinion?

Given the lack of other votes, I pushed this without a volatility column,
and with some other changes --- mostly, I kept the Description column
last, because that's how all the other \d commands do it.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Marko Tiikkaja

marko@joh.to

almost 12 years ago

In reply to: Tom Lane (#7)

Re: Display oprcode and its volatility in \do+

On 1/16/14, 9:32 PM, Tom Lane wrote:

Given the lack of other votes, I pushed this without a volatility column,
and with some other changes --- mostly, I kept the Description column
last, because that's how all the other \d commands do it.

Thanks! And looks like I missed the documentation as well, sorry about
that. :-(

Regards,
Marko Tiikkaja

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Marti Raudsepp

marti@juffo.org

almost 12 years ago

In reply to: Tom Lane (#4)

Re: Display oprcode and its volatility in \do+

On Thu, Jan 16, 2014 at 5:22 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

but adding
volatility here seems like probably a waste of valuable terminal width.
I think that the vast majority of operators have immutable or at worst
stable underlying functions, so this doesn't seem like the first bit
of information I'd need about the underlying function.

For a data point, just today I wanted to look up the volatility of
pg_trgm operators, which made me remember this patch. The \do+ output
is narrow enough, I think an extra volatility column wouldn't be too
bad.

But even just having the function name is a huge improvement, at least
that allows looking up volatility using \commands without accessing
pg_operator directly.

Regards,
Marti

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers