Thread: \d+ should display the storage options for columns

\d+ should display the storage options for columns

From
Gregory Stark
Date:
Oleg pointed out to me here that while we have a command to *set* the toast
storage characteristics there's no actual supported way to display the current
settings.

It seems like this would be a reasonable thing to add to \d+


Index: src/bin/psql/describe.c
===================================================================
RCS file: /home/stark/src/REPOSITORY/pgsql/src/bin/psql/describe.c,v
retrieving revision 1.170
diff -c -r1.170 describe.c
*** src/bin/psql/describe.c    5 May 2008 01:21:03 -0000    1.170
--- src/bin/psql/describe.c    21 May 2008 18:07:13 -0000
***************
*** 865,871 ****

      if (verbose)
      {
!         cols++;
          headers[cols - 1] = _("Description");
      }

--- 865,872 ----

      if (verbose)
      {
!         cols+=2;
!         headers[cols - 2] = _("Storage");
          headers[cols - 1] = _("Description");
      }

***************
*** 877,883 ****
                        "\n  (SELECT substring(pg_catalog.pg_get_expr(d.adbin, d.adrelid) for 128)"
                        "\n   FROM pg_catalog.pg_attrdef d"
                        "\n   WHERE d.adrelid = a.attrelid AND d.adnum = a.attnum AND a.atthasdef),"
!                       "\n  a.attnotnull, a.attnum");
      if (verbose)
          appendPQExpBuffer(&buf, ", pg_catalog.col_description(a.attrelid, a.attnum)");
      appendPQExpBuffer(&buf, "\nFROM pg_catalog.pg_attribute a");
--- 878,884 ----
                        "\n  (SELECT substring(pg_catalog.pg_get_expr(d.adbin, d.adrelid) for 128)"
                        "\n   FROM pg_catalog.pg_attrdef d"
                        "\n   WHERE d.adrelid = a.attrelid AND d.adnum = a.attnum AND a.atthasdef),"
!                       "\n  a.attnotnull, a.attnum, a.attstorage");
      if (verbose)
          appendPQExpBuffer(&buf, ", pg_catalog.col_description(a.attrelid, a.attnum)");
      appendPQExpBuffer(&buf, "\nFROM pg_catalog.pg_attribute a");
***************
*** 957,967 ****
--- 958,979 ----

          /* Description */
          if (verbose)
+         {
+             char *storage = PQgetvalue(res, i, 6);
+
+             cells[i * cols + cols -2] =
+                 (storage[0]=='p' ? "PLAIN" :
+                  (storage[0]=='m' ? "MAIN" :
+                   (storage[0]=='x' ? "EXTENDED" :
+                    (storage[0]=='e' ? "EXTERNAL" :
+                     "???"))));
+
  #ifdef WIN32
              cells[i * cols + cols - 1] = mbvalidate(PQgetvalue(res, i, 5), myopt.encoding);
  #else
              cells[i * cols + cols - 1] = PQgetvalue(res, i, 5);
  #endif
+         }
      }

      /* Make title */


--
  Gregory Stark
  EnterpriseDB          http://www.enterprisedb.com
  Ask me about EnterpriseDB's On-Demand Production Tuning

Re: \d+ should display the storage options for columns

From
Gregory Stark
Date:
"Gregory Stark" <stark@enterprisedb.com> writes:

> Oleg pointed out to me here that while we have a command to *set* the toast
> storage characteristics there's no actual supported way to display the current
> settings.
>
> It seems like this would be a reasonable thing to add to \d+

Sorry, sent the wrong diff before. The previous diff didn't work due to an
array overflow.

Index: src/bin/psql/describe.c
===================================================================
RCS file: /home/stark/src/REPOSITORY/pgsql/src/bin/psql/describe.c,v
retrieving revision 1.170
diff -c -r1.170 describe.c
*** src/bin/psql/describe.c    5 May 2008 01:21:03 -0000    1.170
--- src/bin/psql/describe.c    21 May 2008 20:25:26 -0000
***************
*** 791,797 ****
      printTableOpt myopt = pset.popt.topt;
      int            i;
      char       *view_def = NULL;
!     const char *headers[5];
      char      **cells = NULL;
      char      **footers = NULL;
      char      **ptr;
--- 791,797 ----
      printTableOpt myopt = pset.popt.topt;
      int            i;
      char       *view_def = NULL;
!     const char *headers[6];
      char      **cells = NULL;
      char      **footers = NULL;
      char      **ptr;
***************
*** 865,871 ****

      if (verbose)
      {
!         cols++;
          headers[cols - 1] = _("Description");
      }

--- 865,872 ----

      if (verbose)
      {
!         cols+=2;
!         headers[cols - 2] = _("Storage");
          headers[cols - 1] = _("Description");
      }

***************
*** 877,883 ****
                        "\n  (SELECT substring(pg_catalog.pg_get_expr(d.adbin, d.adrelid) for 128)"
                        "\n   FROM pg_catalog.pg_attrdef d"
                        "\n   WHERE d.adrelid = a.attrelid AND d.adnum = a.attnum AND a.atthasdef),"
!                       "\n  a.attnotnull, a.attnum");
      if (verbose)
          appendPQExpBuffer(&buf, ", pg_catalog.col_description(a.attrelid, a.attnum)");
      appendPQExpBuffer(&buf, "\nFROM pg_catalog.pg_attribute a");
--- 878,884 ----
                        "\n  (SELECT substring(pg_catalog.pg_get_expr(d.adbin, d.adrelid) for 128)"
                        "\n   FROM pg_catalog.pg_attrdef d"
                        "\n   WHERE d.adrelid = a.attrelid AND d.adnum = a.attnum AND a.atthasdef),"
!                       "\n  a.attnotnull, a.attnum, a.attstorage");
      if (verbose)
          appendPQExpBuffer(&buf, ", pg_catalog.col_description(a.attrelid, a.attnum)");
      appendPQExpBuffer(&buf, "\nFROM pg_catalog.pg_attribute a");
***************
*** 957,967 ****

          /* Description */
          if (verbose)
  #ifdef WIN32
!             cells[i * cols + cols - 1] = mbvalidate(PQgetvalue(res, i, 5), myopt.encoding);
  #else
!             cells[i * cols + cols - 1] = PQgetvalue(res, i, 5);
  #endif
      }

      /* Make title */
--- 958,978 ----

          /* Description */
          if (verbose)
+         {
+             char *storage = PQgetvalue(res, i, 5);
+
+             cells[i * cols + cols -2] =
+                 (storage[0]=='p' ? _("plain") :
+                  (storage[0]=='m' ? _("main") :
+                   (storage[0]=='x' ? _("extended") :
+                    (storage[0]=='e' ? _("external") :
+                     "???"))));
  #ifdef WIN32
!             cells[i * cols + cols - 1] = mbvalidate(PQgetvalue(res, i, 6), myopt.encoding);
  #else
!             cells[i * cols + cols - 1] = PQgetvalue(res, i, 6);
  #endif
+         }
      }

      /* Make title */

--
  Gregory Stark
  EnterpriseDB          http://www.enterprisedb.com
  Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL training!

Re: \d+ should display the storage options for columns

From
Alvaro Herrera
Date:
Gregory Stark wrote:
>
> "Gregory Stark" <stark@enterprisedb.com> writes:
>
> > Oleg pointed out to me here that while we have a command to *set* the toast
> > storage characteristics there's no actual supported way to display the current
> > settings.
> >
> > It seems like this would be a reasonable thing to add to \d+
>
> Sorry, sent the wrong diff before. The previous diff didn't work due to an
> array overflow.

This seems to be against an older version of psql ... with the
printTable API stuff, we reworked this -- in particular the mbvalidate()
call that's only on WIN32 is gone (actually it's the lack of it that's
gone.)


--
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Re: \d+ should display the storage options for columns

From
Gregory Stark
Date:
"Alvaro Herrera" <alvherre@commandprompt.com> writes:

> This seems to be against an older version of psql ... with the
> printTable API stuff, we reworked this -- in particular the mbvalidate()
> call that's only on WIN32 is gone (actually it's the lack of it that's
> gone.)

Sorry. Here's a patch against a current sync of HEAD.

Incidentally how can this new API work? Calling _() on a function parameter
would work but how would the translation tools know what strings need to be
translated?


Index: src/bin/psql/describe.c
===================================================================
RCS file: /home/stark/src/REPOSITORY/pgsql/src/bin/psql/describe.c,v
retrieving revision 1.173
diff -c -r1.173 describe.c
*** src/bin/psql/describe.c    13 May 2008 00:23:17 -0000    1.173
--- src/bin/psql/describe.c    23 May 2008 18:52:57 -0000
***************
*** 784,790 ****
      printTableContent cont;
      int            i;
      char       *view_def = NULL;
!     char       *headers[4];
      char      **modifiers = NULL;
      char      **ptr;
      PQExpBufferData title;
--- 784,790 ----
      printTableContent cont;
      int            i;
      char       *view_def = NULL;
!     char       *headers[5];
      char      **modifiers = NULL;
      char      **ptr;
      PQExpBufferData title;
***************
*** 852,858 ****
                        "\n   WHERE d.adrelid = a.attrelid AND d.adnum = a.attnum AND a.atthasdef),"
                        "\n  a.attnotnull, a.attnum");
      if (verbose)
!         appendPQExpBuffer(&buf, ", pg_catalog.col_description(a.attrelid, a.attnum)");
      appendPQExpBuffer(&buf, "\nFROM pg_catalog.pg_attribute a");
      if (tableinfo.relkind == 'i')
          appendPQExpBuffer(&buf, ", pg_catalog.pg_index i");
--- 852,858 ----
                        "\n   WHERE d.adrelid = a.attrelid AND d.adnum = a.attnum AND a.atthasdef),"
                        "\n  a.attnotnull, a.attnum");
      if (verbose)
!         appendPQExpBuffer(&buf, ", a.attstorage, pg_catalog.col_description(a.attrelid, a.attnum)");
      appendPQExpBuffer(&buf, "\nFROM pg_catalog.pg_attribute a");
      if (tableinfo.relkind == 'i')
          appendPQExpBuffer(&buf, ", pg_catalog.pg_index i");
***************
*** 918,924 ****
--- 918,927 ----
      }

      if (verbose)
+     {
+         headers[cols++] = "Storage";
          headers[cols++] = "Description";
+     }

      printTableInit(&cont, &myopt, title.data, cols, numrows);

***************
*** 972,980 ****
              printTableAddCell(&cont, modifiers[i], false);
          }

!         /* Description */
          if (verbose)
!             printTableAddCell(&cont, PQgetvalue(res, i, 5), false);
      }

      /* Make footers */
--- 975,992 ----
              printTableAddCell(&cont, modifiers[i], false);
          }

!         /* Storage and Description */
          if (verbose)
!         {
!             char *storage = PQgetvalue(res, i, 5);
!             printTableAddCell(&cont, (storage[0]=='p' ? "plain" :
!                                       (storage[0]=='m' ? "main" :
!                                        (storage[0]=='x' ? "extended" :
!                                         (storage[0]=='e' ? "external" :
!                                          "???")))),
!                               false);
!             printTableAddCell(&cont, PQgetvalue(res, i, 6), false);
!         }
      }

      /* Make footers */

--
  Gregory Stark
  EnterpriseDB          http://www.enterprisedb.com
  Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL training!

Re: \d+ should display the storage options for columns

From
Alvaro Herrera
Date:
Gregory Stark wrote:
> "Alvaro Herrera" <alvherre@commandprompt.com> writes:
>
> > This seems to be against an older version of psql ... with the
> > printTable API stuff, we reworked this -- in particular the mbvalidate()
> > call that's only on WIN32 is gone (actually it's the lack of it that's
> > gone.)
>
> Sorry. Here's a patch against a current sync of HEAD.

Neat.

> Incidentally how can this new API work? Calling _() on a function parameter
> would work but how would the translation tools know what strings need to be
> translated?

Those strings need to be in gettext_noop().  (If we're missing that
somewhere else, it's a bug) :-)

--
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Re: \d+ should display the storage options for columns

From
Bruce Momjian
Date:
Gregory Stark wrote:
> "Alvaro Herrera" <alvherre@commandprompt.com> writes:
>
> > This seems to be against an older version of psql ... with the
> > printTable API stuff, we reworked this -- in particular the mbvalidate()
> > call that's only on WIN32 is gone (actually it's the lack of it that's
> > gone.)
>
> Sorry. Here's a patch against a current sync of HEAD.
>
> Incidentally how can this new API work? Calling _() on a function parameter
> would work but how would the translation tools know what strings need to be
> translated?

Update patch applied;  I also adjusted some translation function calls.
The new output of psql \d+ is:

    test=> \d+ test
                     Table "public.test"
     Column |  Type   | Modifiers | Storage | Description
    --------+---------+-----------+---------+-------------
     x      | integer |           | plain   |
    Has OIDs: no

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

  + If your life is a hard drive, Christ can be your backup. +
Index: src/bin/psql/describe.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/describe.c,v
retrieving revision 1.177
diff -c -c -r1.177 describe.c
*** src/bin/psql/describe.c    14 Jul 2008 22:00:04 -0000    1.177
--- src/bin/psql/describe.c    14 Jul 2008 22:50:32 -0000
***************
*** 811,817 ****
      printTableContent cont;
      int            i;
      char       *view_def = NULL;
!     char       *headers[4];
      char      **modifiers = NULL;
      char      **ptr;
      PQExpBufferData title;
--- 811,817 ----
      printTableContent cont;
      int            i;
      char       *view_def = NULL;
!     char       *headers[5];
      char      **modifiers = NULL;
      char      **ptr;
      PQExpBufferData title;
***************
*** 878,884 ****
                        "\n   WHERE d.adrelid = a.attrelid AND d.adnum = a.attnum AND a.atthasdef),"
                        "\n  a.attnotnull, a.attnum");
      if (verbose)
!         appendPQExpBuffer(&buf, ", pg_catalog.col_description(a.attrelid, a.attnum)");
      appendPQExpBuffer(&buf, "\nFROM pg_catalog.pg_attribute a");
      if (tableinfo.relkind == 'i')
          appendPQExpBuffer(&buf, ", pg_catalog.pg_index i");
--- 878,884 ----
                        "\n   WHERE d.adrelid = a.attrelid AND d.adnum = a.attnum AND a.atthasdef),"
                        "\n  a.attnotnull, a.attnum");
      if (verbose)
!         appendPQExpBuffer(&buf, ", a.attstorage, pg_catalog.col_description(a.attrelid, a.attnum)");
      appendPQExpBuffer(&buf, "\nFROM pg_catalog.pg_attribute a");
      if (tableinfo.relkind == 'i')
          appendPQExpBuffer(&buf, ", pg_catalog.pg_index i");
***************
*** 933,951 ****

      /* Set the number of columns, and their names */
      cols = 2;
!     headers[0] = "Column";
!     headers[1] = "Type";

      if (tableinfo.relkind == 'r' || tableinfo.relkind == 'v')
      {
          show_modifiers = true;
!         headers[cols++] = "Modifiers";
          modifiers = pg_malloc_zero((numrows + 1) * sizeof(*modifiers));
      }

      if (verbose)
!         headers[cols++] = "Description";
!
      printTableInit(&cont, &myopt, title.data, cols, numrows);

      for (i = 0; i < cols; i++)
--- 933,954 ----

      /* Set the number of columns, and their names */
      cols = 2;
!     headers[0] = gettext_noop("Column");
!     headers[1] = gettext_noop("Type");

      if (tableinfo.relkind == 'r' || tableinfo.relkind == 'v')
      {
          show_modifiers = true;
!         headers[cols++] = gettext_noop("Modifiers");
          modifiers = pg_malloc_zero((numrows + 1) * sizeof(*modifiers));
      }

      if (verbose)
!     {
!         headers[cols++] = gettext_noop("Storage");
!         headers[cols++] = gettext_noop("Description");
!     }
!
      printTableInit(&cont, &myopt, title.data, cols, numrows);

      for (i = 0; i < cols; i++)
***************
*** 1000,1008 ****
              printTableAddCell(&cont, modifiers[i], false);
          }

!         /* Description */
          if (verbose)
!             printTableAddCell(&cont, PQgetvalue(res, i, 5), false);
      }

      /* Make footers */
--- 1003,1020 ----
              printTableAddCell(&cont, modifiers[i], false);
          }

!         /* Storage and Description */
          if (verbose)
!         {
!             char *storage = PQgetvalue(res, i, 5);
!             printTableAddCell(&cont, (storage[0]=='p' ? "plain" :
!                                       (storage[0]=='m' ? "main" :
!                                        (storage[0]=='x' ? "extended" :
!                                         (storage[0]=='e' ? "external" :
!                                          "???")))),
!                               false);
!             printTableAddCell(&cont, PQgetvalue(res, i, 6), false);
!         }
      }

      /* Make footers */

Re: \d+ should display the storage options for columns

From
Alvaro Herrera
Date:
Bruce Momjian wrote:

> Update patch applied;  I also adjusted some translation function calls.
> The new output of psql \d+ is:
>
>     test=> \d+ test
>                      Table "public.test"
>      Column |  Type   | Modifiers | Storage | Description
>     --------+---------+-----------+---------+-------------
>      x      | integer |           | plain   |
>     Has OIDs: no

Thanks for fixing the header problem.

This patch introduces other strings, "plain", "main" and so on that are
not gettext_noop()ed.  Should we mark those for translation too?  I
admit I am not sure, if only because the untranslated strings are what
gets passed to ALTER TABLE ... SET STORAGE.  But if that's the
decision, then it oughtta be commented in the code ...

--
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Re: \d+ should display the storage options for columns

From
Bruce Momjian
Date:
Alvaro Herrera wrote:
> Bruce Momjian wrote:
>
> > Update patch applied;  I also adjusted some translation function calls.
> > The new output of psql \d+ is:
> >
> >     test=> \d+ test
> >                      Table "public.test"
> >      Column |  Type   | Modifiers | Storage | Description
> >     --------+---------+-----------+---------+-------------
> >      x      | integer |           | plain   |
> >     Has OIDs: no
>
> Thanks for fixing the header problem.
>
> This patch introduces other strings, "plain", "main" and so on that are
> not gettext_noop()ed.  Should we mark those for translation too?  I
> admit I am not sure, if only because the untranslated strings are what
> gets passed to ALTER TABLE ... SET STORAGE.  But if that's the
> decision, then it oughtta be commented in the code ...

I thought about that, but those strings are literal used in our syntax,
so translating them seemed wrong.

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

  + If your life is a hard drive, Christ can be your backup. +

Re: \d+ should display the storage options for columns

From
Guillaume Lelarge
Date:
Bruce Momjian a écrit :
> Alvaro Herrera wrote:
>> Bruce Momjian wrote:
>>
>>> Update patch applied;  I also adjusted some translation function calls.
>>> The new output of psql \d+ is:
>>>
>>>     test=> \d+ test
>>>                      Table "public.test"
>>>      Column |  Type   | Modifiers | Storage | Description
>>>     --------+---------+-----------+---------+-------------
>>>      x      | integer |           | plain   |
>>>     Has OIDs: no
>> Thanks for fixing the header problem.
>>
>> This patch introduces other strings, "plain", "main" and so on that are
>> not gettext_noop()ed.  Should we mark those for translation too?  I
>> admit I am not sure, if only because the untranslated strings are what
>> gets passed to ALTER TABLE ... SET STORAGE.  But if that's the
>> decision, then it oughtta be commented in the code ...
>
> I thought about that, but those strings are literal used in our syntax,
> so translating them seemed wrong.
>

I agree on this. If I had to translate these strings, I would copy the
english string in the french translation. I don't see an advantage on
translating these strings.


--
Guillaume.
  http://www.postgresqlfr.org
  http://dalibo.com