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

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

"Gregory Stark" <stark(at)enterprisedb(dot)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!

Gregory Stark wrote:
>
> "Gregory Stark" <stark(at)enterprisedb(dot)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.

"Alvaro Herrera" <alvherre(at)commandprompt(dot)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!

Gregory Stark wrote:
> "Alvaro Herrera" <alvherre(at)commandprompt(dot)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

Gregory Stark wrote:
> "Alvaro Herrera" <alvherre(at)commandprompt(dot)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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

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.

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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

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