Patch by request at pgcon

Started by Greg Starkover 14 years ago7 messages
#1Greg Stark
stark@mit.edu
1 attachment(s)

A user at Pgcon made a pretty convincing case that it would sometimes
be handy to be able to see a list of columns in alphabetical order in
psql's \dt output. This is especially true when there are a very large
number of columns and you're looking for a specific column with name
matching some pattern in your head. Whie you could use your pager's
search feature, people are also quite accustomed to visually searching
for strings in sorted lists so that's a natural solution.

Adding such a feature would be pretty trivial, attached is a patch.

--
greg

Attachments:

psql-sorted-attributes.difftext/x-patch; charset=US-ASCII; name=psql-sorted-attributes.diffDownload
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
***************
*** 1223,1229 **** testdb=>
        <varlistentry>
          <term><literal>\di[S+] [ <link linkend="APP-PSQL-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
          <term><literal>\ds[S+] [ <link linkend="APP-PSQL-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
!         <term><literal>\dt[S+] [ <link linkend="APP-PSQL-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
          <term><literal>\dv[S+] [ <link linkend="APP-PSQL-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
          <term><literal>\dE[S+] [ <link linkend="APP-PSQL-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
  
--- 1223,1229 ----
        <varlistentry>
          <term><literal>\di[S+] [ <link linkend="APP-PSQL-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
          <term><literal>\ds[S+] [ <link linkend="APP-PSQL-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
!         <term><literal>\dt[S+<] [ <link linkend="APP-PSQL-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
          <term><literal>\dv[S+] [ <link linkend="APP-PSQL-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
          <term><literal>\dE[S+] [ <link linkend="APP-PSQL-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
  
***************
*** 1237,1246 **** testdb=&gt;
          You can specify any or all of
          these letters, in any order, to obtain a listing of objects
          of these types.  For example, <literal>\dit</> lists indexes
!         and tables.  If <literal>+</literal> is
!         appended to the command name, each object is listed with its
!         physical size on disk and its associated description, if any.
!         If <replaceable class="parameter">pattern</replaceable> is
          specified, only objects whose names match the pattern are listed.
          By default, only user-created objects are shown; supply a
          pattern or the <literal>S</literal> modifier to include system
--- 1237,1253 ----
          You can specify any or all of
          these letters, in any order, to obtain a listing of objects
          of these types.  For example, <literal>\dit</> lists indexes
!         and tables.  
! 	<para>
!         If <literal>+</literal> is appended to the command name, each
!         object is listed with its physical size on disk and its
!         associated description, if any.  If <literal>&lt;</literal> is
!         appended to <literal>\dt</literal> command then attributes of
!         tables are listed in alphabetical order (using the database's
!         default collation) rather than in the order they appear in the
!         table definition.
! 	<para>
! 	If <replaceable class="parameter">pattern</replaceable> is
          specified, only objects whose names match the pattern are listed.
          By default, only user-created objects are shown; supply a
          pattern or the <literal>S</literal> modifier to include system
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
***************
*** 351,357 **** exec_command(const char *cmd,
  	{
  		char	   *pattern;
  		bool		show_verbose,
! 					show_system;
  
  		/* We don't do SQLID reduction on the pattern yet */
  		pattern = psql_scan_slash_option(scan_state,
--- 351,358 ----
  	{
  		char	   *pattern;
  		bool		show_verbose,
! 					show_system,
! 					sorted_list;
  
  		/* We don't do SQLID reduction on the pattern yet */
  		pattern = psql_scan_slash_option(scan_state,
***************
*** 359,372 **** exec_command(const char *cmd,
  
  		show_verbose = strchr(cmd, '+') ? true : false;
  		show_system = strchr(cmd, 'S') ? true : false;
  
  		switch (cmd[1])
  		{
  			case '\0':
  			case '+':
  			case 'S':
  				if (pattern)
! 					success = describeTableDetails(pattern, show_verbose, show_system);
  				else
  					/* standard listing of interesting things */
  					success = listTables("tvsE", NULL, show_verbose, show_system);
--- 360,375 ----
  
  		show_verbose = strchr(cmd, '+') ? true : false;
  		show_system = strchr(cmd, 'S') ? true : false;
+ 		sorted_list = strchr(cmd, '<') ? true : false;
  
  		switch (cmd[1])
  		{
  			case '\0':
  			case '+':
  			case 'S':
+ 			case '<':
  				if (pattern)
! 					success = describeTableDetails(pattern, show_verbose, show_system, sorted_list);
  				else
  					/* standard listing of interesting things */
  					success = listTables("tvsE", NULL, show_verbose, show_system);
*** a/src/bin/psql/describe.c
--- b/src/bin/psql/describe.c
***************
*** 26,32 ****
  static bool describeOneTableDetails(const char *schemaname,
  						const char *relationname,
  						const char *oid,
! 						bool verbose);
  static void add_tablespace_footer(printTableContent *const cont, char relkind,
  					  Oid tablespace, const bool newline);
  static void add_role_attribute(PQExpBuffer buf, const char *const str);
--- 26,33 ----
  static bool describeOneTableDetails(const char *schemaname,
  						const char *relationname,
  						const char *oid,
! 						bool verbose,
! 						bool sortedAttrs);
  static void add_tablespace_footer(printTableContent *const cont, char relkind,
  					  Oid tablespace, const bool newline);
  static void add_role_attribute(PQExpBuffer buf, const char *const str);
***************
*** 1030,1036 **** objectDescription(const char *pattern, bool showSystem)
   * verbose: if true, this is \d+
   */
  bool
! describeTableDetails(const char *pattern, bool verbose, bool showSystem)
  {
  	PQExpBufferData buf;
  	PGresult   *res;
--- 1031,1037 ----
   * verbose: if true, this is \d+
   */
  bool
! describeTableDetails(const char *pattern, bool verbose, bool showSystem, bool sortedAttrs)
  {
  	PQExpBufferData buf;
  	PGresult   *res;
***************
*** 1079,1085 **** describeTableDetails(const char *pattern, bool verbose, bool showSystem)
  		nspname = PQgetvalue(res, i, 1);
  		relname = PQgetvalue(res, i, 2);
  
! 		if (!describeOneTableDetails(nspname, relname, oid, verbose))
  		{
  			PQclear(res);
  			return false;
--- 1080,1086 ----
  		nspname = PQgetvalue(res, i, 1);
  		relname = PQgetvalue(res, i, 2);
  
! 		if (!describeOneTableDetails(nspname, relname, oid, verbose, sortedAttrs))
  		{
  			PQclear(res);
  			return false;
***************
*** 1106,1112 **** static bool
  describeOneTableDetails(const char *schemaname,
  						const char *relationname,
  						const char *oid,
! 						bool verbose)
  {
  	PQExpBufferData buf;
  	PGresult   *res = NULL;
--- 1107,1114 ----
  describeOneTableDetails(const char *schemaname,
  						const char *relationname,
  						const char *oid,
! 						bool verbose,
! 						bool sortedAttrs)
  {
  	PQExpBufferData buf;
  	PGresult   *res = NULL;
***************
*** 1299,1305 **** describeOneTableDetails(const char *schemaname,
  		appendPQExpBuffer(&buf, ",\n  a.attstorage, pg_catalog.col_description(a.attrelid, a.attnum)");
  	appendPQExpBuffer(&buf, "\nFROM pg_catalog.pg_attribute a");
  	appendPQExpBuffer(&buf, "\nWHERE a.attrelid = '%s' AND a.attnum > 0 AND NOT a.attisdropped", oid);
! 	appendPQExpBuffer(&buf, "\nORDER BY a.attnum");
  
  	res = PSQLexec(buf.data, false);
  	if (!res)
--- 1301,1310 ----
  		appendPQExpBuffer(&buf, ",\n  a.attstorage, pg_catalog.col_description(a.attrelid, a.attnum)");
  	appendPQExpBuffer(&buf, "\nFROM pg_catalog.pg_attribute a");
  	appendPQExpBuffer(&buf, "\nWHERE a.attrelid = '%s' AND a.attnum > 0 AND NOT a.attisdropped", oid);
! 	if (!sortedAttrs)
! 		appendPQExpBuffer(&buf, "\nORDER BY a.attnum");
! 	else
! 		appendPQExpBuffer(&buf, "\nORDER BY a.attname");
  
  	res = PSQLexec(buf.data, false);
  	if (!res)
*** a/src/bin/psql/describe.h
--- b/src/bin/psql/describe.h
***************
*** 40,46 **** extern bool listDefaultACLs(const char *pattern);
  extern bool objectDescription(const char *pattern, bool showSystem);
  
  /* \d foo */
! extern bool describeTableDetails(const char *pattern, bool verbose, bool showSystem);
  
  /* \dF */
  extern bool listTSConfigs(const char *pattern, bool verbose);
--- 40,46 ----
  extern bool objectDescription(const char *pattern, bool showSystem);
  
  /* \d foo */
! extern bool describeTableDetails(const char *pattern, bool verbose, bool showSystem, bool sortedAttrs);
  
  /* \dF */
  extern bool listTSConfigs(const char *pattern, bool verbose);
#2Alvaro Herrera
alvherre@commandprompt.com
In reply to: Greg Stark (#1)
Re: Patch by request at pgcon

Excerpts from Greg Stark's message of jue may 19 12:11:29 -0400 2011:

A user at Pgcon made a pretty convincing case that it would sometimes
be handy to be able to see a list of columns in alphabetical order in
psql's \dt output. This is especially true when there are a very large
number of columns and you're looking for a specific column with name
matching some pattern in your head. Whie you could use your pager's
search feature, people are also quite accustomed to visually searching
for strings in sorted lists so that's a natural solution.

Adding such a feature would be pretty trivial, attached is a patch.

Interesting, but not so trivial I think -- I mean if you're doing this I
think you should add a column with the nominal position of the column in
the table, so that it enables you to find it quickly in the other sort
order.

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

#3Stephen Frost
sfrost@snowman.net
In reply to: Alvaro Herrera (#2)
Re: Patch by request at pgcon

* Alvaro Herrera (alvherre@commandprompt.com) wrote:

Excerpts from Greg Stark's message of jue may 19 12:11:29 -0400 2011:

Adding such a feature would be pretty trivial, attached is a patch.

Interesting, but not so trivial I think -- I mean if you're doing this I
think you should add a column with the nominal position of the column in
the table, so that it enables you to find it quickly in the other sort
order.

Afraid that I have to disagree.. The attnum (or, really, worse, since
you'd have to actually count/number the non-dropped columns only..)
doesn't strike me as being useful to the user for much of anything,
especially since we don't have the number anywhere in the default
listing.

Thanks,

Stephen

#4Stephen Frost
sfrost@snowman.net
In reply to: Greg Stark (#1)
Re: Patch by request at pgcon

* Greg Stark (stark@mit.edu) wrote:

Adding such a feature would be pretty trivial, attached is a patch.

Quick code-only review: sortedAttrs is a horrible var name for what it
is. More like 'sortbyName' would make more sense, at least to me.
Also, no regression test..? :) The interface looks alright to me,
though I wonder for a moment if this might be a \set option. Are there
other things which could be sorted differently based on such an
option..? If not, then perhaps another char for \dt is right.

Thanks,

Stephen

#5Alvaro Herrera
alvherre@commandprompt.com
In reply to: Stephen Frost (#3)
Re: Patch by request at pgcon

Excerpts from Stephen Frost's message of jue may 19 13:49:33 -0400 2011:

* Alvaro Herrera (alvherre@commandprompt.com) wrote:

Excerpts from Greg Stark's message of jue may 19 12:11:29 -0400 2011:

Adding such a feature would be pretty trivial, attached is a patch.

Interesting, but not so trivial I think -- I mean if you're doing this I
think you should add a column with the nominal position of the column in
the table, so that it enables you to find it quickly in the other sort
order.

Afraid that I have to disagree.. The attnum (or, really, worse, since
you'd have to actually count/number the non-dropped columns only..)
doesn't strike me as being useful to the user for much of anything,
especially since we don't have the number anywhere in the default
listing.

Well, counting is not that hard (I mean if there's more than a
screenful, just watch the pager). But yeah, it's not the attnum.

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

#6Greg Stark
stark@mit.edu
In reply to: Stephen Frost (#4)
Re: Patch by request at pgcon

On Thu, May 19, 2011 at 1:53 PM, Stephen Frost <sfrost@snowman.net> wrote:

Quick code-only review: sortedAttrs is a horrible var name for what it
is.  More like 'sortbyName' would make more sense, at least to me.
Also, no regression test..? :)

Do we have regression tests for psql \ commands?

--
greg

#7Stephen Frost
sfrost@snowman.net
In reply to: Greg Stark (#6)
Re: Patch by request at pgcon

* Greg Stark (stark@mit.edu) wrote:

On Thu, May 19, 2011 at 1:53 PM, Stephen Frost <sfrost@snowman.net> wrote:

Quick code-only review: sortedAttrs is a horrible var name for what it
is.  More like 'sortbyName' would make more sense, at least to me.
Also, no regression test..? :)

Do we have regression tests for psql \ commands?

There's certainly lots of usage of psql \ commands in the regression
tests, though we don't have a dedicated "test psql \ commands"
regression suite. Suppose it's a toss-up on if we should add this/other
ones. My comment was more toungue-in-cheek as I stressed regression
tests in my talk this morning. :)

Stephen