pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feed[PATCH v1] Show whether tables are logged in \dt+
9+ messages / 4 participants
[nested] [flat]
* [PATCH v1] Show whether tables are logged in \dt+
@ 2019-04-23 00:50 David Fetter <david.fetter@onelogin.com>
0 siblings, 0 replies; 9+ messages in thread
From: David Fetter @ 2019-04-23 00:50 UTC (permalink / raw)
\d would show this for individual tables, but there wasn't an
overarching view of all tables. Now, there is.
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index ee00c5da08..5ef567c123 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3631,7 +3631,7 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
PQExpBufferData buf;
PGresult *res;
printQueryOpt myopt = pset.popt;
- static const bool translate_columns[] = {false, false, true, false, false, false, false};
+ static const bool translate_columns[] = {false, false, true, false, false, false, false, false};
/* If tabtypes is empty, we default to \dtvmsE (but see also command.c) */
if (!(showTables || showIndexes || showViews || showMatViews || showSeq || showForeign))
@@ -3695,6 +3695,11 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
appendPQExpBuffer(&buf,
",\n pg_catalog.obj_description(c.oid, 'pg_class') as \"%s\"",
gettext_noop("Description"));
+
+ if (pset.sversion >= 91000)
+ appendPQExpBuffer(&buf,
+ ",\n c.relpersistence <> 'u' as \"%s\"",
+ gettext_noop("Logged"));
}
appendPQExpBufferStr(&buf,
^ permalink raw reply [nested|flat] 9+ messages in thread
* [PATCH v1] Show whether tables are logged in \dt+
@ 2019-04-23 00:56 David Fetter <david@fetter.org>
2019-04-23 05:03 ` Re: [PATCH v1] Show whether tables are logged in \dt+ Fabien COELHO <coelho@cri.ensmp.fr>
0 siblings, 1 reply; 9+ messages in thread
From: David Fetter @ 2019-04-23 00:56 UTC (permalink / raw)
To: pgsql-hackers
Folks,
I noticed that there wasn't a bulk way to see table logged-ness in
psql, so I made it part of \dt+.
What say?
Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
Attachments:
[text/x-diff] v1-0001-Show-whether-tables-are-logged-in-dt.patch (1.7K, ../../20190423005642.GZ28936@fetter.org/2-v1-0001-Show-whether-tables-are-logged-in-dt.patch)
download | inline diff:
From 518efc003730c441663aae42c18d16f0908bd55d Mon Sep 17 00:00:00 2001
From: David Fetter <david.fetter@onelogin.com>
Date: Mon, 22 Apr 2019 17:50:48 -0700
Subject: [PATCH v1] Show whether tables are logged in \dt+
To: hackers
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------2.20.1"
This is a multi-part message in MIME format.
--------------2.20.1
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit
\d would show this for individual tables, but there wasn't an
overarching view of all tables. Now, there is.
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index ee00c5da08..5ef567c123 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3631,7 +3631,7 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
PQExpBufferData buf;
PGresult *res;
printQueryOpt myopt = pset.popt;
- static const bool translate_columns[] = {false, false, true, false, false, false, false};
+ static const bool translate_columns[] = {false, false, true, false, false, false, false, false};
/* If tabtypes is empty, we default to \dtvmsE (but see also command.c) */
if (!(showTables || showIndexes || showViews || showMatViews || showSeq || showForeign))
@@ -3695,6 +3695,11 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
appendPQExpBuffer(&buf,
",\n pg_catalog.obj_description(c.oid, 'pg_class') as \"%s\"",
gettext_noop("Description"));
+
+ if (pset.sversion >= 91000)
+ appendPQExpBuffer(&buf,
+ ",\n c.relpersistence <> 'u' as \"%s\"",
+ gettext_noop("Logged"));
}
appendPQExpBufferStr(&buf,
--------------2.20.1--
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: [PATCH v1] Show whether tables are logged in \dt+
2019-04-23 00:56 [PATCH v1] Show whether tables are logged in \dt+ David Fetter <david@fetter.org>
@ 2019-04-23 05:03 ` Fabien COELHO <coelho@cri.ensmp.fr>
2019-04-24 06:26 ` Re: [PATCH v1] Show whether tables are logged in \dt+ David Fetter <david@fetter.org>
0 siblings, 1 reply; 9+ messages in thread
From: Fabien COELHO @ 2019-04-23 05:03 UTC (permalink / raw)
To: David Fetter <david@fetter.org>; +Cc: pgsql-hackers
Hello David,
> I noticed that there wasn't a bulk way to see table logged-ness in psql,
> so I made it part of \dt+.
Applies, compiles, works for me.
ISTM That temporary-ness is not shown either. Maybe the persistence column
should be shown as is?
Also I'd suggest that the column should be displayed before the
"description" column to keep the length-varying one last?
> What say?
Tests? Doc?
--
Fabien.
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: [PATCH v1] Show whether tables are logged in \dt+
2019-04-23 00:56 [PATCH v1] Show whether tables are logged in \dt+ David Fetter <david@fetter.org>
2019-04-23 05:03 ` Re: [PATCH v1] Show whether tables are logged in \dt+ Fabien COELHO <coelho@cri.ensmp.fr>
@ 2019-04-24 06:26 ` David Fetter <david@fetter.org>
2019-04-24 08:29 ` Re: [PATCH v1] Show whether tables are logged in \dt+ Fabien COELHO <coelho@cri.ensmp.fr>
0 siblings, 1 reply; 9+ messages in thread
From: David Fetter @ 2019-04-24 06:26 UTC (permalink / raw)
To: Fabien COELHO <coelho@cri.ensmp.fr>; +Cc: pgsql-hackers
On Tue, Apr 23, 2019 at 07:03:58AM +0200, Fabien COELHO wrote:
>
> Hello David,
>
> > I noticed that there wasn't a bulk way to see table logged-ness in psql,
> > so I made it part of \dt+.
>
> Applies, compiles, works for me.
>
> ISTM That temporary-ness is not shown either. Maybe the persistence column
> should be shown as is?
Temporariness added, but not raw.
> Also I'd suggest that the column should be displayed before the
> "description" column to keep the length-varying one last?
Done.
> > What say?
>
> Tests?
Included, but they're not stable for temp tables. I'm a little stumped
as to how to either stabilize them or test some other way.
> Doc?
What further documentation does it need?
Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
Attachments:
[text/x-diff] v2-0001-Show-table-persistence-in-dt.patch (2.6K, ../../20190424062623.GF28936@fetter.org/2-v2-0001-Show-table-persistence-in-dt.patch)
download | inline diff:
From 9e82160c2fe2f554e3417b3fbff147bc6024e3fe Mon Sep 17 00:00:00 2001
From: David Fetter <david.fetter@onelogin.com>
Date: Mon, 22 Apr 2019 17:50:48 -0700
Subject: [PATCH v2] Show table persistence in \dt+
To: hackers
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------2.20.1"
This is a multi-part message in MIME format.
--------------2.20.1
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit
\d would show this for individual tables, but there wasn't an
overarching view of all tables. Now, there is.
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 5ef567c123..02e7eb0862 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3679,6 +3679,13 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
if (verbose)
{
+ /*
+ * Show whether the table is permanent, temporary, or unlogged.
+ */
+ if (pset.sversion >= 91000)
+ appendPQExpBuffer(&buf,
+ ",\n case c.relpersistence when 'p' then 'permanent' when 't' then 'temporary' when 'u' then 'unlogged' else 'unknown' end as \"%s\"",
+ gettext_noop("Persistence"));
/*
* As of PostgreSQL 9.0, use pg_table_size() to show a more accurate
* size of a table, including FSM, VM and TOAST tables.
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 35856bffdd..f4f468804f 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -4729,3 +4729,16 @@ drop schema testpart;
set search_path to default;
set role to default;
drop role testrole_partitioning;
+create table foo(id integer);
+create temp table tfoo(id integer);
+create unlogged table ufoo(id integer);
+\dt+ *.*foo
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Size | Description
+-----------+------+-------+---------+-------------+---------+-------------
+ pg_temp_3 | tfoo | table | shackle | temporary | 0 bytes |
+ public | foo | table | shackle | permanent | 0 bytes |
+ public | ufoo | table | shackle | unlogged | 0 bytes |
+(3 rows)
+
+drop table foo, tfoo, ufoo;
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 78f4b5d7d5..c9ad6ffd9c 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -1115,3 +1115,10 @@ set search_path to default;
set role to default;
drop role testrole_partitioning;
+
+set search_path = public, pg_temp;
+create table foo(id integer);
+create temp table tfoo(id integer);
+create unlogged table ufoo(id integer);
+\dt+ *.*foo
+drop table foo, tfoo, ufoo;
--------------2.20.1--
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: [PATCH v1] Show whether tables are logged in \dt+
2019-04-23 00:56 [PATCH v1] Show whether tables are logged in \dt+ David Fetter <david@fetter.org>
2019-04-23 05:03 ` Re: [PATCH v1] Show whether tables are logged in \dt+ Fabien COELHO <coelho@cri.ensmp.fr>
2019-04-24 06:26 ` Re: [PATCH v1] Show whether tables are logged in \dt+ David Fetter <david@fetter.org>
@ 2019-04-24 08:29 ` Fabien COELHO <coelho@cri.ensmp.fr>
2019-04-26 12:49 ` Re: [PATCH v1] Show whether tables are logged in \dt+ Rafia Sabih <rafia.pghackers@gmail.com>
0 siblings, 1 reply; 9+ messages in thread
From: Fabien COELHO @ 2019-04-24 08:29 UTC (permalink / raw)
To: David Fetter <david@fetter.org>; +Cc: pgsql-hackers
Hello David,
>>> I noticed that there wasn't a bulk way to see table logged-ness in psql,
>>> so I made it part of \dt+.
>>
>> Applies, compiles, works for me.
>>
>> ISTM That temporary-ness is not shown either. Maybe the persistence column
>> should be shown as is?
>
> Temporariness added, but not raw.
Ok, it is better like this way.
>> Tests?
>
> Included, but they're not stable for temp tables. I'm a little stumped
> as to how to either stabilize them or test some other way.
Hmmm. First there is the username which appears, so there should be a
dedicated user for the test.
I'm unsure how to work around the temporary schema number, which is
undeterministic with parallel execution it. I'm afraid the only viable
approach is not to show temporary tables, too bad:-(
>> Doc?
>
> What further documentation does it need?
Indeed, there is no precise doc, so nothing to update :-)/:-(
Maybe you could consider adding a case for prior 9.1 version, something
like:
... case c.relistemp then 'temporary' else 'permanent' end as ...
--
Fabien.
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: [PATCH v1] Show whether tables are logged in \dt+
2019-04-23 00:56 [PATCH v1] Show whether tables are logged in \dt+ David Fetter <david@fetter.org>
2019-04-23 05:03 ` Re: [PATCH v1] Show whether tables are logged in \dt+ Fabien COELHO <coelho@cri.ensmp.fr>
2019-04-24 06:26 ` Re: [PATCH v1] Show whether tables are logged in \dt+ David Fetter <david@fetter.org>
2019-04-24 08:29 ` Re: [PATCH v1] Show whether tables are logged in \dt+ Fabien COELHO <coelho@cri.ensmp.fr>
@ 2019-04-26 12:49 ` Rafia Sabih <rafia.pghackers@gmail.com>
2019-04-26 14:22 ` Re: [PATCH v1] Show whether tables are logged in \dt+ Rafia Sabih <rafia.pghackers@gmail.com>
0 siblings, 1 reply; 9+ messages in thread
From: Rafia Sabih @ 2019-04-26 12:49 UTC (permalink / raw)
To: Fabien COELHO <coelho@cri.ensmp.fr>; +Cc: David Fetter <david@fetter.org>; pgsql-hackers
On Wed, 24 Apr 2019 at 10:30, Fabien COELHO <coelho@cri.ensmp.fr> wrote:
>
>
> Hello David,
>
> >>> I noticed that there wasn't a bulk way to see table logged-ness in psql,
> >>> so I made it part of \dt+.
> >>
> >> Applies, compiles, works for me.
> >>
> >> ISTM That temporary-ness is not shown either. Maybe the persistence column
> >> should be shown as is?
> >
> > Temporariness added, but not raw.
>
> Ok, it is better like this way.
>
> >> Tests?
> >
> > Included, but they're not stable for temp tables. I'm a little stumped
> > as to how to either stabilize them or test some other way.
>
> Hmmm. First there is the username which appears, so there should be a
> dedicated user for the test.
>
> I'm unsure how to work around the temporary schema number, which is
> undeterministic with parallel execution it. I'm afraid the only viable
> approach is not to show temporary tables, too bad:-(
>
> >> Doc?
> >
> > What further documentation does it need?
>
> Indeed, there is no precise doc, so nothing to update :-)/:-(
>
>
> Maybe you could consider adding a case for prior 9.1 version, something
> like:
> ... case c.relistemp then 'temporary' else 'permanent' end as ...
>
>
I was reviewing this patch and found a bug,
create table t (i int);
create index idx on t(i);
\di+
psql: print.c:3452: printQuery: Assertion `opt->translate_columns ==
((void *)0) || opt->n_translate_columns >= cont.ncolumns' failed.
--
Regards,
Rafia Sabih
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: [PATCH v1] Show whether tables are logged in \dt+
2019-04-23 00:56 [PATCH v1] Show whether tables are logged in \dt+ David Fetter <david@fetter.org>
2019-04-23 05:03 ` Re: [PATCH v1] Show whether tables are logged in \dt+ Fabien COELHO <coelho@cri.ensmp.fr>
2019-04-24 06:26 ` Re: [PATCH v1] Show whether tables are logged in \dt+ David Fetter <david@fetter.org>
2019-04-24 08:29 ` Re: [PATCH v1] Show whether tables are logged in \dt+ Fabien COELHO <coelho@cri.ensmp.fr>
2019-04-26 12:49 ` Re: [PATCH v1] Show whether tables are logged in \dt+ Rafia Sabih <rafia.pghackers@gmail.com>
@ 2019-04-26 14:22 ` Rafia Sabih <rafia.pghackers@gmail.com>
2019-04-27 04:18 ` Re: [PATCH v1] Show whether tables are logged in \dt+ David Fetter <david@fetter.org>
0 siblings, 1 reply; 9+ messages in thread
From: Rafia Sabih @ 2019-04-26 14:22 UTC (permalink / raw)
To: Fabien COELHO <coelho@cri.ensmp.fr>; +Cc: David Fetter <david@fetter.org>; pgsql-hackers
On Fri, 26 Apr 2019 at 14:49, Rafia Sabih <rafia.pghackers@gmail.com> wrote:
>
> On Wed, 24 Apr 2019 at 10:30, Fabien COELHO <coelho@cri.ensmp.fr> wrote:
> >
> >
> > Hello David,
> >
> > >>> I noticed that there wasn't a bulk way to see table logged-ness in psql,
> > >>> so I made it part of \dt+.
> > >>
> > >> Applies, compiles, works for me.
> > >>
> > >> ISTM That temporary-ness is not shown either. Maybe the persistence column
> > >> should be shown as is?
> > >
> > > Temporariness added, but not raw.
> >
> > Ok, it is better like this way.
> >
> > >> Tests?
> > >
> > > Included, but they're not stable for temp tables. I'm a little stumped
> > > as to how to either stabilize them or test some other way.
> >
> > Hmmm. First there is the username which appears, so there should be a
> > dedicated user for the test.
> >
> > I'm unsure how to work around the temporary schema number, which is
> > undeterministic with parallel execution it. I'm afraid the only viable
> > approach is not to show temporary tables, too bad:-(
> >
> > >> Doc?
> > >
> > > What further documentation does it need?
> >
> > Indeed, there is no precise doc, so nothing to update :-)/:-(
> >
> >
> > Maybe you could consider adding a case for prior 9.1 version, something
> > like:
> > ... case c.relistemp then 'temporary' else 'permanent' end as ...
> >
> >
> I was reviewing this patch and found a bug,
>
> create table t (i int);
> create index idx on t(i);
> \di+
> psql: print.c:3452: printQuery: Assertion `opt->translate_columns ==
> ((void *)0) || opt->n_translate_columns >= cont.ncolumns' failed.
Looking into this further, apparently the position of
if (verbose)
{
+ /*
+ * Show whether the table is permanent, temporary, or unlogged.
+ */
+ if (pset.sversion >= 91000)
+ appendPQExpBuffer(&buf,
+ ",\n case c.relpersistence when 'p' then 'permanent' when 't'
then 'temporary' when 'u' then 'unlogged' else 'unknown' end as
\"%s\"",
+ gettext_noop("Persistence"));
is not right, it is being called for indexes with verbose option also.
There should be an extra check for it being not called for index case.
Something like,
if (verbose)
{
/*
* Show whether the table is permanent, temporary, or unlogged.
*/
if (!showIndexes)
if (pset.sversion >= 91000)
appendPQExpBuffer(&buf,
",\n case c.relpersistence when 'p' then 'permanent' when 't' then
'temporary' when 'u' then 'unlogged' else 'unknown' end as \"%s\"",
gettext_noop("Persistence"));
Not sure, how do modify it in a more neat way.
--
Regards,
Rafia Sabih
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: [PATCH v1] Show whether tables are logged in \dt+
2019-04-23 00:56 [PATCH v1] Show whether tables are logged in \dt+ David Fetter <david@fetter.org>
2019-04-23 05:03 ` Re: [PATCH v1] Show whether tables are logged in \dt+ Fabien COELHO <coelho@cri.ensmp.fr>
2019-04-24 06:26 ` Re: [PATCH v1] Show whether tables are logged in \dt+ David Fetter <david@fetter.org>
2019-04-24 08:29 ` Re: [PATCH v1] Show whether tables are logged in \dt+ Fabien COELHO <coelho@cri.ensmp.fr>
2019-04-26 12:49 ` Re: [PATCH v1] Show whether tables are logged in \dt+ Rafia Sabih <rafia.pghackers@gmail.com>
2019-04-26 14:22 ` Re: [PATCH v1] Show whether tables are logged in \dt+ Rafia Sabih <rafia.pghackers@gmail.com>
@ 2019-04-27 04:18 ` David Fetter <david@fetter.org>
2019-05-03 07:44 ` Re: [PATCH v1] Show whether tables are logged in \dt+ Rafia Sabih <rafia.pghackers@gmail.com>
0 siblings, 1 reply; 9+ messages in thread
From: David Fetter @ 2019-04-27 04:18 UTC (permalink / raw)
To: Rafia Sabih <rafia.pghackers@gmail.com>; +Cc: Fabien COELHO <coelho@cri.ensmp.fr>; pgsql-hackers
On Fri, Apr 26, 2019 at 04:22:18PM +0200, Rafia Sabih wrote:
> On Fri, 26 Apr 2019 at 14:49, Rafia Sabih <rafia.pghackers@gmail.com> wrote:
> >
> > On Wed, 24 Apr 2019 at 10:30, Fabien COELHO <coelho@cri.ensmp.fr> wrote:
> > >
> > >
> > > Hello David,
> > >
> > > >>> I noticed that there wasn't a bulk way to see table logged-ness in psql,
> > > >>> so I made it part of \dt+.
> > > >>
> > > >> Applies, compiles, works for me.
> > > >>
> > > >> ISTM That temporary-ness is not shown either. Maybe the persistence column
> > > >> should be shown as is?
> > > >
> > > > Temporariness added, but not raw.
> > >
> > > Ok, it is better like this way.
> > >
> > > >> Tests?
> > > >
> > > > Included, but they're not stable for temp tables. I'm a little stumped
> > > > as to how to either stabilize them or test some other way.
> > >
> > > Hmmm. First there is the username which appears, so there should be a
> > > dedicated user for the test.
> > >
> > > I'm unsure how to work around the temporary schema number, which is
> > > undeterministic with parallel execution it. I'm afraid the only viable
> > > approach is not to show temporary tables, too bad:-(
> > >
> > > >> Doc?
> > > >
> > > > What further documentation does it need?
> > >
> > > Indeed, there is no precise doc, so nothing to update :-)/:-(
> > >
> > >
> > > Maybe you could consider adding a case for prior 9.1 version, something
> > > like:
> > > ... case c.relistemp then 'temporary' else 'permanent' end as ...
> > >
> > >
> > I was reviewing this patch and found a bug,
> >
> > create table t (i int);
> > create index idx on t(i);
> > \di+
> > psql: print.c:3452: printQuery: Assertion `opt->translate_columns ==
> > ((void *)0) || opt->n_translate_columns >= cont.ncolumns' failed.
>
> Looking into this further, apparently the position of
>
> if (verbose)
> {
> + /*
> + * Show whether the table is permanent, temporary, or unlogged.
> + */
> + if (pset.sversion >= 91000)
> + appendPQExpBuffer(&buf,
> + ",\n case c.relpersistence when 'p' then 'permanent' when 't'
> then 'temporary' when 'u' then 'unlogged' else 'unknown' end as
> \"%s\"",
> + gettext_noop("Persistence"));
>
> is not right, it is being called for indexes with verbose option also.
> There should be an extra check for it being not called for index case.
> Something like,
> if (verbose)
> {
> /*
> * Show whether the table is permanent, temporary, or unlogged.
> */
> if (!showIndexes)
> if (pset.sversion >= 91000)
> appendPQExpBuffer(&buf,
> ",\n case c.relpersistence when 'p' then 'permanent' when 't' then
> 'temporary' when 'u' then 'unlogged' else 'unknown' end as \"%s\"",
> gettext_noop("Persistence"));
>
> Not sure, how do modify it in a more neat way.
I suspect that as this may get a little messier, but I've made it
fairly neat short of a major refactor.
Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
Attachments:
[text/x-diff] v3-0001-Show-detailed-table-persistence-in-dt.patch (4.4K, ../../20190427041849.GL28936@fetter.org/2-v3-0001-Show-detailed-table-persistence-in-dt.patch)
download | inline diff:
From c5bd317b9904fe017afa5489b11a3cbae6e80f3b Mon Sep 17 00:00:00 2001
From: David Fetter <david.fetter@onelogin.com>
Date: Mon, 22 Apr 2019 17:50:48 -0700
Subject: [PATCH v3] Show detailed table persistence in \dt+
To: hackers
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------2.20.1"
This is a multi-part message in MIME format.
--------------2.20.1
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit
\d would show this for individual tables, but there wasn't an
overarching view of all tables. Now, there is.
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index ee00c5da08..620e7d69bf 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3631,7 +3631,7 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
PQExpBufferData buf;
PGresult *res;
printQueryOpt myopt = pset.popt;
- static const bool translate_columns[] = {false, false, true, false, false, false, false};
+ static const bool translate_columns[] = {false, false, true, false, false, false, false, false};
/* If tabtypes is empty, we default to \dtvmsE (but see also command.c) */
if (!(showTables || showIndexes || showViews || showMatViews || showSeq || showForeign))
@@ -3680,17 +3680,32 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
if (verbose)
{
/*
- * As of PostgreSQL 9.0, use pg_table_size() to show a more accurate
- * size of a table, including FSM, VM and TOAST tables.
+ * Show whether a table is permanent, temporary, or unlogged.
+ * Indexes are not, as of this writing, tables.
*/
- if (pset.sversion >= 90000)
- appendPQExpBuffer(&buf,
- ",\n pg_catalog.pg_size_pretty(pg_catalog.pg_table_size(c.oid)) as \"%s\"",
- gettext_noop("Size"));
- else if (pset.sversion >= 80100)
- appendPQExpBuffer(&buf,
- ",\n pg_catalog.pg_size_pretty(pg_catalog.pg_relation_size(c.oid)) as \"%s\"",
- gettext_noop("Size"));
+ if (!showIndexes)
+ {
+ if (pset.sversion >= 91000)
+ appendPQExpBuffer(&buf,
+ ",\n case c.relpersistence when 'p' then 'permanent' when 't' then 'temporary' when 'u' then 'unlogged' else 'unknown' end as \"%s\"",
+ gettext_noop("Persistence"));
+ else
+ appendPQExpBuffer(&buf,
+ ",\n case when c.relistemp then 'temporary' else 'permanent' end as \"%s\"",
+ gettext_noop("Persistence"));
+ }
+ /*
+ * As of PostgreSQL 9.0, use pg_table_size() to show a more accurate
+ * size of a table, including FSM, VM and TOAST tables.
+ */
+ if (pset.sversion >= 90000)
+ appendPQExpBuffer(&buf,
+ ",\n pg_catalog.pg_size_pretty(pg_catalog.pg_table_size(c.oid)) as \"%s\"",
+ gettext_noop("Size"));
+ else if (pset.sversion >= 80100)
+ appendPQExpBuffer(&buf,
+ ",\n pg_catalog.pg_size_pretty(pg_catalog.pg_relation_size(c.oid)) as \"%s\"",
+ gettext_noop("Size"));
appendPQExpBuffer(&buf,
",\n pg_catalog.obj_description(c.oid, 'pg_class') as \"%s\"",
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 35856bffdd..f4f468804f 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -4729,3 +4729,16 @@ drop schema testpart;
set search_path to default;
set role to default;
drop role testrole_partitioning;
+create table foo(id integer);
+create temp table tfoo(id integer);
+create unlogged table ufoo(id integer);
+\dt+ *.*foo
+ List of relations
+ Schema | Name | Type | Owner | Persistence | Size | Description
+-----------+------+-------+---------+-------------+---------+-------------
+ pg_temp_3 | tfoo | table | shackle | temporary | 0 bytes |
+ public | foo | table | shackle | permanent | 0 bytes |
+ public | ufoo | table | shackle | unlogged | 0 bytes |
+(3 rows)
+
+drop table foo, tfoo, ufoo;
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 78f4b5d7d5..c9ad6ffd9c 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -1115,3 +1115,10 @@ set search_path to default;
set role to default;
drop role testrole_partitioning;
+
+set search_path = public, pg_temp;
+create table foo(id integer);
+create temp table tfoo(id integer);
+create unlogged table ufoo(id integer);
+\dt+ *.*foo
+drop table foo, tfoo, ufoo;
--------------2.20.1--
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: [PATCH v1] Show whether tables are logged in \dt+
2019-04-23 00:56 [PATCH v1] Show whether tables are logged in \dt+ David Fetter <david@fetter.org>
2019-04-23 05:03 ` Re: [PATCH v1] Show whether tables are logged in \dt+ Fabien COELHO <coelho@cri.ensmp.fr>
2019-04-24 06:26 ` Re: [PATCH v1] Show whether tables are logged in \dt+ David Fetter <david@fetter.org>
2019-04-24 08:29 ` Re: [PATCH v1] Show whether tables are logged in \dt+ Fabien COELHO <coelho@cri.ensmp.fr>
2019-04-26 12:49 ` Re: [PATCH v1] Show whether tables are logged in \dt+ Rafia Sabih <rafia.pghackers@gmail.com>
2019-04-26 14:22 ` Re: [PATCH v1] Show whether tables are logged in \dt+ Rafia Sabih <rafia.pghackers@gmail.com>
2019-04-27 04:18 ` Re: [PATCH v1] Show whether tables are logged in \dt+ David Fetter <david@fetter.org>
@ 2019-05-03 07:44 ` Rafia Sabih <rafia.pghackers@gmail.com>
0 siblings, 0 replies; 9+ messages in thread
From: Rafia Sabih @ 2019-05-03 07:44 UTC (permalink / raw)
To: David Fetter <david@fetter.org>; +Cc: Fabien COELHO <coelho@cri.ensmp.fr>; pgsql-hackers
On Sat, 27 Apr 2019 at 06:18, David Fetter <david@fetter.org> wrote:
>
> On Fri, Apr 26, 2019 at 04:22:18PM +0200, Rafia Sabih wrote:
> > On Fri, 26 Apr 2019 at 14:49, Rafia Sabih <rafia.pghackers@gmail.com> wrote:
> > >
> > > On Wed, 24 Apr 2019 at 10:30, Fabien COELHO <coelho@cri.ensmp.fr> wrote:
> > > >
> > > >
> > > > Hello David,
> > > >
> > > > >>> I noticed that there wasn't a bulk way to see table logged-ness in psql,
> > > > >>> so I made it part of \dt+.
> > > > >>
> > > > >> Applies, compiles, works for me.
> > > > >>
> > > > >> ISTM That temporary-ness is not shown either. Maybe the persistence column
> > > > >> should be shown as is?
> > > > >
> > > > > Temporariness added, but not raw.
> > > >
> > > > Ok, it is better like this way.
> > > >
> > > > >> Tests?
> > > > >
> > > > > Included, but they're not stable for temp tables. I'm a little stumped
> > > > > as to how to either stabilize them or test some other way.
> > > >
> > > > Hmmm. First there is the username which appears, so there should be a
> > > > dedicated user for the test.
> > > >
> > > > I'm unsure how to work around the temporary schema number, which is
> > > > undeterministic with parallel execution it. I'm afraid the only viable
> > > > approach is not to show temporary tables, too bad:-(
> > > >
> > > > >> Doc?
> > > > >
> > > > > What further documentation does it need?
> > > >
> > > > Indeed, there is no precise doc, so nothing to update :-)/:-(
> > > >
> > > >
> > > > Maybe you could consider adding a case for prior 9.1 version, something
> > > > like:
> > > > ... case c.relistemp then 'temporary' else 'permanent' end as ...
> > > >
> > > >
> > > I was reviewing this patch and found a bug,
> > >
> > > create table t (i int);
> > > create index idx on t(i);
> > > \di+
> > > psql: print.c:3452: printQuery: Assertion `opt->translate_columns ==
> > > ((void *)0) || opt->n_translate_columns >= cont.ncolumns' failed.
> >
> > Looking into this further, apparently the position of
> >
> > if (verbose)
> > {
> > + /*
> > + * Show whether the table is permanent, temporary, or unlogged.
> > + */
> > + if (pset.sversion >= 91000)
> > + appendPQExpBuffer(&buf,
> > + ",\n case c.relpersistence when 'p' then 'permanent' when 't'
> > then 'temporary' when 'u' then 'unlogged' else 'unknown' end as
> > \"%s\"",
> > + gettext_noop("Persistence"));
> >
> > is not right, it is being called for indexes with verbose option also.
> > There should be an extra check for it being not called for index case.
> > Something like,
> > if (verbose)
> > {
> > /*
> > * Show whether the table is permanent, temporary, or unlogged.
> > */
> > if (!showIndexes)
> > if (pset.sversion >= 91000)
> > appendPQExpBuffer(&buf,
> > ",\n case c.relpersistence when 'p' then 'permanent' when 't' then
> > 'temporary' when 'u' then 'unlogged' else 'unknown' end as \"%s\"",
> > gettext_noop("Persistence"));
> >
> > Not sure, how do modify it in a more neat way.
>
> I suspect that as this may get a little messier, but I've made it
> fairly neat short of a major refactor.
>
I found the following warning on the compilation,
describe.c: In function ‘listTables’:
describe.c:3705:7: warning: this ‘if’ clause does not guard...
[-Wmisleading-indentation]
else if (pset.sversion >= 80100)
^~
describe.c:3710:3: note: ...this statement, but the latter is
misleadingly indented as if it were guarded by the ‘if’
appendPQExpBuffer(&buf,
Talking of indentation, you might want to run pgindent once. Other
than that the patch looks good to me.
--
Regards,
Rafia Sabih
^ permalink raw reply [nested|flat] 9+ messages in thread
end of thread, other threads:[~2019-05-03 07:44 UTC | newest]
Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-04-23 00:50 [PATCH v1] Show whether tables are logged in \dt+ David Fetter <david.fetter@onelogin.com>
2019-04-23 00:56 [PATCH v1] Show whether tables are logged in \dt+ David Fetter <david@fetter.org>
2019-04-23 05:03 ` Fabien COELHO <coelho@cri.ensmp.fr>
2019-04-24 06:26 ` David Fetter <david@fetter.org>
2019-04-24 08:29 ` Fabien COELHO <coelho@cri.ensmp.fr>
2019-04-26 12:49 ` Rafia Sabih <rafia.pghackers@gmail.com>
2019-04-26 14:22 ` Rafia Sabih <rafia.pghackers@gmail.com>
2019-04-27 04:18 ` David Fetter <david@fetter.org>
2019-05-03 07:44 ` Rafia Sabih <rafia.pghackers@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox