psql \d sequence display
In PostgreSQL 10, the sequence metadata moved from the sequence
"relation" to a system catalog. The psql \d sequence command was not
updated for that. (It just did SELECT * FROM seq and there were no
tests, so this was missed.) Attached is a patch that fixes that up,
taking the opportunity to design a more useful sequence display that is
not merely hacked on to the general relation display.
This should be fixed for PG10, so if you have any feedback on the
design, please let me know soon.
Examples are in the attached patch.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-psql-Update-d-sequence-display.patchtext/plain; charset=UTF-8; name=0001-psql-Update-d-sequence-display.patch; x-mac-creator=0; x-mac-type=0Download
From 506cc57e751fa76ac5b5ce298faaa98e5e6462d4 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Mon, 25 Sep 2017 11:59:46 -0400
Subject: [PATCH] psql: Update \d sequence display
For \d sequencename, the psql code just did SELECT * FROM sequencename
to get the information to display, but this does not contain much
interesting information anymore in PostgreSQL 10, because the metadata
has been moved to a separate system catalog.
This patch creates a newly designed sequence display that is not merely
an extension of the general relation/table display as it was previously.
Example:
PostgreSQL 9.6:
=> \d foobar
Sequence "public.foobar"
Column | Type | Value
---------------+---------+---------------------
sequence_name | name | foobar
last_value | bigint | 1
start_value | bigint | 1
increment_by | bigint | 1
max_value | bigint | 9223372036854775807
min_value | bigint | 1
cache_value | bigint | 1
log_cnt | bigint | 0
is_cycled | boolean | f
is_called | boolean | f
PostgreSQL 10 before this change:
=> \d foobar
Sequence "public.foobar"
Column | Type | Value
------------+---------+-------
last_value | bigint | 1
log_cnt | bigint | 0
is_called | boolean | f
New:
=> \d foobar
Sequence "public.foobar"
Type | Start | Minimum | Maximum | Increment | Cycles | Cache
--------+-------+---------+---------------------+-----------+--------+-------
bigint | 1 | 1 | 9223372036854775807 | 1 | no | 1
---
src/bin/psql/describe.c | 189 +++++++++++++++++++--------------
src/test/regress/expected/identity.out | 7 ++
src/test/regress/expected/sequence.out | 13 +++
src/test/regress/sql/identity.sql | 2 +
src/test/regress/sql/sequence.sql | 4 +
5 files changed, 135 insertions(+), 80 deletions(-)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 798e71045f..2a5c4408e8 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1380,8 +1380,6 @@ describeOneTableDetails(const char *schemaname,
int i;
char *view_def = NULL;
char *headers[11];
- char **seq_values = NULL;
- char **ptr;
PQExpBufferData title;
PQExpBufferData tmpbuf;
int cols;
@@ -1563,27 +1561,125 @@ describeOneTableDetails(const char *schemaname,
res = NULL;
/*
- * If it's a sequence, fetch its values and store into an array that will
- * be used later.
+ * If it's a sequence, deal with it here separately.
*/
if (tableinfo.relkind == RELKIND_SEQUENCE)
{
- printfPQExpBuffer(&buf, "SELECT * FROM %s", fmtId(schemaname));
- /* must be separate because fmtId isn't reentrant */
- appendPQExpBuffer(&buf, ".%s;", fmtId(relationname));
+ PGresult *result = NULL;
+ printQueryOpt myopt = pset.popt;
+ char *footers[2] = {NULL, NULL};
+
+ if (pset.sversion >= 100000)
+ {
+ printfPQExpBuffer(&buf,
+ "SELECT pg_catalog.format_type(seqtypid, NULL) AS \"%s\",\n"
+ " seqstart AS \"%s\",\n"
+ " seqmin AS \"%s\",\n"
+ " seqmax AS \"%s\",\n"
+ " seqincrement AS \"%s\",\n"
+ " CASE WHEN seqcycle THEN '%s' ELSE '%s' END AS \"%s\",\n"
+ " seqcache AS \"%s\"\n",
+ gettext_noop("Type"),
+ gettext_noop("Start"),
+ gettext_noop("Minimum"),
+ gettext_noop("Maximum"),
+ gettext_noop("Increment"),
+ gettext_noop("yes"),
+ gettext_noop("no"),
+ gettext_noop("Cycles"),
+ gettext_noop("Cache"));
+ appendPQExpBuffer(&buf,
+ "FROM pg_catalog.pg_sequence\n"
+ "WHERE seqrelid = '%s';",
+ oid);
+ }
+ else
+ {
+ printfPQExpBuffer(&buf,
+ "SELECT pg_catalog.format_type('bigint'::regtype, NULL) AS \"%s\",\n"
+ " start_value AS \"%s\",\n"
+ " min_value AS \"%s\",\n"
+ " max_value AS \"%s\",\n"
+ " increment_by AS \"%s\",\n"
+ " CASE WHEN is_cycled THEN '%s' ELSE '%s' END AS \"%s\",\n"
+ " cache_value AS \"%s\"\n",
+ gettext_noop("Type"),
+ gettext_noop("Start"),
+ gettext_noop("Minimum"),
+ gettext_noop("Maximum"),
+ gettext_noop("Increment"),
+ gettext_noop("yes"),
+ gettext_noop("no"),
+ gettext_noop("Cycles"),
+ gettext_noop("Cache"));
+ appendPQExpBuffer(&buf, "FROM %s", fmtId(schemaname));
+ /* must be separate because fmtId isn't reentrant */
+ appendPQExpBuffer(&buf, ".%s;", fmtId(relationname));
+ }
res = PSQLexec(buf.data);
if (!res)
goto error_return;
- seq_values = pg_malloc((PQnfields(res) + 1) * sizeof(*seq_values));
+ /* Footer information about a sequence */
+
+ /* Get the column that owns this sequence */
+ printfPQExpBuffer(&buf, "SELECT pg_catalog.quote_ident(nspname) || '.' ||"
+ "\n pg_catalog.quote_ident(relname) || '.' ||"
+ "\n pg_catalog.quote_ident(attname),"
+ "\n d.deptype"
+ "\nFROM pg_catalog.pg_class c"
+ "\nINNER JOIN pg_catalog.pg_depend d ON c.oid=d.refobjid"
+ "\nINNER JOIN pg_catalog.pg_namespace n ON n.oid=c.relnamespace"
+ "\nINNER JOIN pg_catalog.pg_attribute a ON ("
+ "\n a.attrelid=c.oid AND"
+ "\n a.attnum=d.refobjsubid)"
+ "\nWHERE d.classid='pg_catalog.pg_class'::pg_catalog.regclass"
+ "\n AND d.refclassid='pg_catalog.pg_class'::pg_catalog.regclass"
+ "\n AND d.objid='%s'"
+ "\n AND d.deptype IN ('a', 'i')",
+ oid);
+
+ result = PSQLexec(buf.data);
+
+ /*
+ * If we get no rows back, don't show anything (obviously). We should
+ * never get more than one row back, but if we do, just ignore it and
+ * don't print anything.
+ */
+ if (!result)
+ goto error_return;
+ else if (PQntuples(result) == 1)
+ {
+ switch (PQgetvalue(result, 0, 1)[0])
+ {
+ case 'a':
+ footers[0] = psprintf(_("Owned by: %s"),
+ PQgetvalue(result, 0, 0));
+ break;
+ case 'i':
+ footers[0] = psprintf(_("Sequence for identity column: %s"),
+ PQgetvalue(result, 0, 0));
+ break;
+ }
+ }
+ PQclear(result);
+
+ printfPQExpBuffer(&title, _("Sequence \"%s.%s\""),
+ schemaname, relationname);
- for (i = 0; i < PQnfields(res); i++)
- seq_values[i] = pg_strdup(PQgetvalue(res, 0, i));
- seq_values[i] = NULL;
+ myopt.footers = footers;
+ myopt.topt.default_footer = false;
+ myopt.title = title.data;
+ myopt.translate_header = true;
- PQclear(res);
- res = NULL;
+ printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+
+ if (footers[0])
+ free(footers[0]);
+
+ retval = true;
+ goto error_return; /* not an error, just return early */
}
/*
@@ -1667,10 +1763,6 @@ describeOneTableDetails(const char *schemaname,
printfPQExpBuffer(&title, _("Materialized view \"%s.%s\""),
schemaname, relationname);
break;
- case RELKIND_SEQUENCE:
- printfPQExpBuffer(&title, _("Sequence \"%s.%s\""),
- schemaname, relationname);
- break;
case RELKIND_INDEX:
if (tableinfo.relpersistence == 'u')
printfPQExpBuffer(&title, _("Unlogged index \"%s.%s\""),
@@ -1729,9 +1821,6 @@ describeOneTableDetails(const char *schemaname,
show_column_details = true;
}
- if (tableinfo.relkind == RELKIND_SEQUENCE)
- headers[cols++] = gettext_noop("Value");
-
if (tableinfo.relkind == RELKIND_INDEX)
headers[cols++] = gettext_noop("Definition");
@@ -1813,10 +1902,6 @@ describeOneTableDetails(const char *schemaname,
printTableAddCell(&cont, default_str, false, false);
}
- /* Value: for sequences only */
- if (tableinfo.relkind == RELKIND_SEQUENCE)
- printTableAddCell(&cont, seq_values[i], false, false);
-
/* Expression for index column */
if (tableinfo.relkind == RELKIND_INDEX)
printTableAddCell(&cont, PQgetvalue(res, i, 7), false, false);
@@ -2027,55 +2112,6 @@ describeOneTableDetails(const char *schemaname,
PQclear(result);
}
- else if (tableinfo.relkind == RELKIND_SEQUENCE)
- {
- /* Footer information about a sequence */
- PGresult *result = NULL;
-
- /* Get the column that owns this sequence */
- printfPQExpBuffer(&buf, "SELECT pg_catalog.quote_ident(nspname) || '.' ||"
- "\n pg_catalog.quote_ident(relname) || '.' ||"
- "\n pg_catalog.quote_ident(attname),"
- "\n d.deptype"
- "\nFROM pg_catalog.pg_class c"
- "\nINNER JOIN pg_catalog.pg_depend d ON c.oid=d.refobjid"
- "\nINNER JOIN pg_catalog.pg_namespace n ON n.oid=c.relnamespace"
- "\nINNER JOIN pg_catalog.pg_attribute a ON ("
- "\n a.attrelid=c.oid AND"
- "\n a.attnum=d.refobjsubid)"
- "\nWHERE d.classid='pg_catalog.pg_class'::pg_catalog.regclass"
- "\n AND d.refclassid='pg_catalog.pg_class'::pg_catalog.regclass"
- "\n AND d.objid='%s'"
- "\n AND d.deptype IN ('a', 'i')",
- oid);
-
- result = PSQLexec(buf.data);
- if (!result)
- goto error_return;
- else if (PQntuples(result) == 1)
- {
- switch (PQgetvalue(result, 0, 1)[0])
- {
- case 'a':
- printfPQExpBuffer(&buf, _("Owned by: %s"),
- PQgetvalue(result, 0, 0));
- printTableAddFooter(&cont, buf.data);
- break;
- case 'i':
- printfPQExpBuffer(&buf, _("Sequence for identity column: %s"),
- PQgetvalue(result, 0, 0));
- printTableAddFooter(&cont, buf.data);
- break;
- }
- }
-
- /*
- * If we get no rows back, don't show anything (obviously). We should
- * never get more than one row back, but if we do, just ignore it and
- * don't print anything.
- */
- PQclear(result);
- }
else if (tableinfo.relkind == RELKIND_RELATION ||
tableinfo.relkind == RELKIND_MATVIEW ||
tableinfo.relkind == RELKIND_FOREIGN_TABLE ||
@@ -2960,13 +2996,6 @@ describeOneTableDetails(const char *schemaname,
termPQExpBuffer(&title);
termPQExpBuffer(&tmpbuf);
- if (seq_values)
- {
- for (ptr = seq_values; *ptr; ptr++)
- free(*ptr);
- free(seq_values);
- }
-
if (view_def)
free(view_def);
diff --git a/src/test/regress/expected/identity.out b/src/test/regress/expected/identity.out
index 2800ed7caa..aa5317baeb 100644
--- a/src/test/regress/expected/identity.out
+++ b/src/test/regress/expected/identity.out
@@ -32,6 +32,13 @@ SELECT pg_get_serial_sequence('itest1', 'a');
public.itest1_a_seq
(1 row)
+\d itest1_a_seq
+ Sequence "public.itest1_a_seq"
+ Type | Start | Minimum | Maximum | Increment | Cycles | Cache
+---------+-------+---------+------------+-----------+--------+-------
+ integer | 1 | 1 | 2147483647 | 1 | no | 1
+Sequence for identity column: public.itest1.a
+
CREATE TABLE itest4 (a int, b text);
ALTER TABLE itest4 ALTER COLUMN a ADD GENERATED ALWAYS AS IDENTITY; -- error, requires NOT NULL
ERROR: column "a" of relation "itest4" must be declared NOT NULL before identity can be added
diff --git a/src/test/regress/expected/sequence.out b/src/test/regress/expected/sequence.out
index ea05a3382b..f72d7f94b6 100644
--- a/src/test/regress/expected/sequence.out
+++ b/src/test/regress/expected/sequence.out
@@ -535,6 +535,19 @@ SELECT * FROM pg_sequence_parameters('sequence_test4'::regclass);
-1 | -9223372036854775808 | -1 | -1 | f | 1 | 20
(1 row)
+\d sequence_test4
+ Sequence "public.sequence_test4"
+ Type | Start | Minimum | Maximum | Increment | Cycles | Cache
+--------+-------+----------------------+---------+-----------+--------+-------
+ bigint | -1 | -9223372036854775808 | -1 | -1 | no | 1
+
+\d serialtest2_f2_seq
+ Sequence "public.serialtest2_f2_seq"
+ Type | Start | Minimum | Maximum | Increment | Cycles | Cache
+---------+-------+---------+------------+-----------+--------+-------
+ integer | 1 | 1 | 2147483647 | 1 | no | 1
+Owned by: public.serialtest2.f2
+
-- Test comments
COMMENT ON SEQUENCE asdf IS 'won''t work';
ERROR: relation "asdf" does not exist
diff --git a/src/test/regress/sql/identity.sql b/src/test/regress/sql/identity.sql
index 7886456a56..e1b5a074c9 100644
--- a/src/test/regress/sql/identity.sql
+++ b/src/test/regress/sql/identity.sql
@@ -14,6 +14,8 @@ CREATE TABLE itest3 (a smallint generated by default as identity (start with 7 i
SELECT pg_get_serial_sequence('itest1', 'a');
+\d itest1_a_seq
+
CREATE TABLE itest4 (a int, b text);
ALTER TABLE itest4 ALTER COLUMN a ADD GENERATED ALWAYS AS IDENTITY; -- error, requires NOT NULL
ALTER TABLE itest4 ALTER COLUMN a SET NOT NULL;
diff --git a/src/test/regress/sql/sequence.sql b/src/test/regress/sql/sequence.sql
index c50834a5b9..a7b9e63372 100644
--- a/src/test/regress/sql/sequence.sql
+++ b/src/test/regress/sql/sequence.sql
@@ -246,6 +246,10 @@ CREATE SEQUENCE sequence_test3; -- not read from, to test is_called
SELECT * FROM pg_sequence_parameters('sequence_test4'::regclass);
+\d sequence_test4
+\d serialtest2_f2_seq
+
+
-- Test comments
COMMENT ON SEQUENCE asdf IS 'won''t work';
COMMENT ON SEQUENCE sequence_test2 IS 'will work';
base-commit: 99e90bac4f9f3bd8d7b285a6f4095c2089e09efe
--
2.14.1
Hello,
This should be fixed for PG10, so if you have any feedback on the
design, please let me know soon.
Works for me on head against a 9.6 server, which is good.
My 0.02 €:
\d+ does not show more.
Maybe Type, Min, Max, Inc & Cycles are enough for \d?
The next/future or last/previous value is not shown. If one could be
available it would be nice to have?
Maybe some names are a little large, eg "Increment" could be "Inc.".
The value is nearly always 1?
Not sure why it is "Cycles" (plural) instead of "Cycle".
The non regression test could also show a more esoteric sequence (cyclic,
...).
There is no documentation. Maybe none is needed.
I do not understand why some queries have "... \n" "... \n" and others
have "\n ..." "\n ...". I would suggest to homogeneize around the former,
because "\nFROM ..." is less readable.
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 9/25/17 13:53, Fabien COELHO wrote:
\d+ does not show more.
Maybe Type, Min, Max, Inc & Cycles are enough for \d?
That seems kind of arbitrary. Start and Cache are just as relevant.
The next/future or last/previous value is not shown. If one could be
available it would be nice to have?
You can get those from the sequence. Running \d on a table doesn't show
the table data either. So I think this is a valid distinction.
Maybe some names are a little large, eg "Increment" could be "Inc.".
The value is nearly always 1?
Yeah, but then that would look weird if only one heading were
abbreviated. The total display width is good, so we don't need to
squeeze it too hard.
Not sure why it is "Cycles" (plural) instead of "Cycle".
This is meant to be a verb, as in "does it cycle?". I have changed it
to "Cycles?" to match other similar headings.
I do not understand why some queries have "... \n" "... \n" and others
have "\n ..." "\n ...". I would suggest to homogeneize around the former,
because "\nFROM ..." is less readable.
Yeah that is a bit ugly, but I have just moved existing code around.
I have now committed my patch with minor adjustments, so we have
something working for PG10.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers