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>
  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 05:03  Fabien COELHO <coelho@cri.ensmp.fr>
  parent: 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-24 06:26  David Fetter <david@fetter.org>
  parent: 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-24 08:29  Fabien COELHO <coelho@cri.ensmp.fr>
  parent: David Fetter <david@fetter.org>
  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-26 12:49  Rafia Sabih <rafia.pghackers@gmail.com>
  parent: Fabien COELHO <coelho@cri.ensmp.fr>
  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-26 14:22  Rafia Sabih <rafia.pghackers@gmail.com>
  parent: Rafia Sabih <rafia.pghackers@gmail.com>
  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-27 04:18  David Fetter <david@fetter.org>
  parent: 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-05-03 07:44  Rafia Sabih <rafia.pghackers@gmail.com>
  parent: David Fetter <david@fetter.org>
  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