Code review for patch to show definition of index columns in \d on index.

Safely schema-qualify the pg_get_indexdef call, make the query a bit
prettier in -E mode, remove useless join to pg_index, make it more obvious
that the header[] array is not overrun.

diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index d176685fdc4fa916a6fb59e31e7341c040d5d256..3f1b402418b255f5c32048cd5b1c3ce1f366476e 100644 (file)
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1028,7 +1028,7 @@ describeOneTableDetails(const char *schemaname,
        char      **ptr;
        PQExpBufferData title;
        PQExpBufferData tmpbuf;
-       int                     cols = 0;
+       int                     cols;
        int                     numrows = 0;
        struct
        {
@@ -1156,7 +1156,7 @@ describeOneTableDetails(const char *schemaname,
                PQclear(result);
        }
 
-       /* Get column info (index requires additional checks) */
+       /* Get column info */
        printfPQExpBuffer(&buf, "SELECT a.attname,");
        appendPQExpBuffer(&buf, "\n  pg_catalog.format_type(a.atttypid, a.atttypmod),"
                                          "\n  (SELECT substring(pg_catalog.pg_get_expr(d.adbin, d.adrelid) for 128)"
@@ -1164,15 +1164,11 @@ describeOneTableDetails(const char *schemaname,
                                          "\n   WHERE d.adrelid = a.attrelid AND d.adnum = a.attnum AND a.atthasdef),"
                                          "\n  a.attnotnull, a.attnum");
        if (tableinfo.relkind == 'i')
-               appendPQExpBuffer(&buf, ", pg_get_indexdef(i.indexrelid,a.attnum, TRUE) AS indexdef");
+               appendPQExpBuffer(&buf, ",\n  pg_catalog.pg_get_indexdef(a.attrelid, a.attnum, TRUE) AS indexdef");
        if (verbose)
-               appendPQExpBuffer(&buf, ", a.attstorage, pg_catalog.col_description(a.attrelid, a.attnum)");
+               appendPQExpBuffer(&buf, ",\n  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");
        appendPQExpBuffer(&buf, "\nWHERE a.attrelid = '%s' AND a.attnum > 0 AND NOT a.attisdropped", oid);
-       if (tableinfo.relkind == 'i')
-               appendPQExpBuffer(&buf, " AND a.attrelid = i.indexrelid");
        appendPQExpBuffer(&buf, "\nORDER BY a.attnum");
 
        res = PSQLexec(buf.data, false);
@@ -1220,9 +1216,9 @@ describeOneTableDetails(const char *schemaname,
        }
 
        /* Set the number of columns, and their names */
-       cols += 2;
        headers[0] = gettext_noop("Column");
        headers[1] = gettext_noop("Type");
+       cols = 2;
 
        if (tableinfo.relkind == 'r' || tableinfo.relkind == 'v')
        {
@@ -1302,15 +1298,15 @@ describeOneTableDetails(const char *schemaname,
                if (tableinfo.relkind == 'S')
                        printTableAddCell(&cont, seq_values[i], false);
 
-               /* Expression for index */
+               /* Expression for index column */
                if (tableinfo.relkind == 'i')
                        printTableAddCell(&cont, PQgetvalue(res, i, 5), false);
 
                /* Storage and Description */
                if (verbose)
                {
-                       int fnum = (tableinfo.relkind == 'i' ? 6 : 5);
-                       char       *storage  = PQgetvalue(res, i, fnum);
+                       int                     firstvcol = (tableinfo.relkind == 'i' ? 6 : 5);
+                       char       *storage  = PQgetvalue(res, i, firstvcol);
 
                        /* these strings are literal in our syntax, so not translated. */
                        printTableAddCell(&cont, (storage[0] == 'p' ? "plain" :
@@ -1319,7 +1315,7 @@ describeOneTableDetails(const char *schemaname,
                                                                                (storage[0] == 'e' ? "external" :
                                                                                 "???")))),
                                                          false);
-                       printTableAddCell(&cont, PQgetvalue(res, i, fnum + 1), false);
+                       printTableAddCell(&cont, PQgetvalue(res, i, firstvcol + 1), false);
                }
        }
 
@@ -1844,20 +1840,23 @@ describeOneTableDetails(const char *schemaname,
                }
                else
                {
-                       /* display the list of child tables*/
+                       /* display the list of child tables */
+                       const char *ct = _("Child tables");
+
                        for (i = 0; i < tuples; i++)
-                       {
-                               const char *ct = _("Child tables");
-
-                               if (i == 0)
-                                       printfPQExpBuffer(&buf, "%s: %s", ct, PQgetvalue(result, i, 0));
-                               else
-                                       printfPQExpBuffer(&buf, "%*s  %s", (int) strlen(ct), "", PQgetvalue(result, i, 0));
-                               if (i < tuples - 1)
-                                       appendPQExpBuffer(&buf, ",");
-
-                               printTableAddFooter(&cont, buf.data);
-                       }
+                       {
+                               if (i == 0)
+                                       printfPQExpBuffer(&buf, "%s: %s",
+                                                                         ct, PQgetvalue(result, i, 0));
+                               else
+                                       printfPQExpBuffer(&buf, "%*s  %s",
+                                                                         (int) strlen(ct), "",
+                                                                         PQgetvalue(result, i, 0));
+                               if (i < tuples - 1)
+                                       appendPQExpBuffer(&buf, ",");
+
+                               printTableAddFooter(&cont, buf.data);
+                       }
                }
                PQclear(result);