pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feednRe: [PATCH v1] Show whether tables are logged in \dt+
3+ messages / 2 participants
[nested] [flat]
* nRe: [PATCH v1] Show whether tables are logged in \dt+
@ 2019-04-27 07:19 Fabien COELHO <coelho@cri.ensmp.fr>
0 siblings, 1 reply; 3+ messages in thread
From: Fabien COELHO @ 2019-04-27 07:19 UTC (permalink / raw)
To: David Fetter <david@fetter.org>; +Cc: Rafia Sabih <rafia.pghackers@gmail.com>; pgsql-hackers
Hello David,
Patch v3 applies, but compiles for me with a warning because the
indentation of the following size block has been changed:
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,
^~~~~~~~~~~~~~~~~
Make check fails because of my temp schema was numbered 4 instead of 3,
and I'm "fabien" rather than "shackle".
>>>>> 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:-(
The tests have not been fixed.
I think that they need a dedicated user to replace "shackle", and I'm
afraid that there temporary test schema instability cannot be easily fixed
at the "psql" level, but would require some kind of TAP tests instead if
it is to be checked. In the short term, do not.
I checked that the \di+ works, though. I've played with temporary views
and \dv as well.
I discovered that you cannot have temporary unlogged objects, nor
temporary or unlogged materialized views. Intuitively I'd have thought
that these features would be orthogonal, but they are not. Also I created
an unlogged table with a SERIAL which created a sequence. The table is
unlogged but the sequence is permanent, which is probably ok.
I only have packages down to pg 9.3, so I could not test prior 9.1. By
looking at the online documentation, is seems that relistemp appears in pg
8.4, so the corresponding extraction should be guarded by this version.
Before that, temporary objects existed but were identified indirectly,
possibly because they were stored in a temporary schema. I suggest not to
try to address cases prior 8.4.
--
Fabien.
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: nRe: [PATCH v1] Show whether tables are logged in \dt+
@ 2019-04-27 17:43 David Fetter <david@fetter.org>
parent: Fabien COELHO <coelho@cri.ensmp.fr>
0 siblings, 1 reply; 3+ messages in thread
From: David Fetter @ 2019-04-27 17:43 UTC (permalink / raw)
To: Fabien COELHO <coelho@cri.ensmp.fr>; +Cc: Rafia Sabih <rafia.pghackers@gmail.com>; pgsql-hackers
On Sat, Apr 27, 2019 at 09:19:57AM +0200, Fabien COELHO wrote:
>
> Hello David,
>
> Patch v3 applies, but compiles for me with a warning because the indentation
> of the following size block has been changed:
>
> 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,
> ^~~~~~~~~~~~~~~~~
Fixed.
> Make check fails because of my temp schema was numbered 4 instead of 3, and
> I'm "fabien" rather than "shackle".
I think the way forward is to test this with TAP rather than the
fixed-string method.
> > > > > > 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:-(
>
> The tests have not been fixed.
>
> I think that they need a dedicated user to replace "shackle", and I'm afraid
> that there temporary test schema instability cannot be easily fixed at the
> "psql" level, but would require some kind of TAP tests instead if it is to
> be checked. In the short term, do not.
Checks removed while I figure out a new TAP test.
> I checked that the \di+ works, though. I've played with temporary views and
> \dv as well.
Great!
> I discovered that you cannot have temporary unlogged objects, nor
> temporary or unlogged materialized views. Intuitively I'd have
> thought that these features would be orthogonal, but they are not.
This seems like material for a different patch.
> Also I created an unlogged table with a SERIAL which created a
> sequence. The table is unlogged but the sequence is permanent, which
> is probably ok.
> I only have packages down to pg 9.3, so I could not test prior 9.1.
> By looking at the online documentation, is seems that relistemp
> appears in pg 8.4, so the corresponding extraction should be guarded
> by this version. Before that, temporary objects existed but were
> identified indirectly, possibly because they were stored in a
> temporary schema. I suggest not to try to address cases prior 8.4.
Done.
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:
v4-0001-Show-detailed-table-persistence-in-dt.patch (text/x-diff, 3.3K) download
From fd0b8215ca6bbdebc0924efaba95944731890dc9 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 v4] 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..4aa06a417c 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,22 +3680,36 @@ 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 if (pset.sversion >= 84000)
+ 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\"",
- gettext_noop("Description"));
- }
+ appendPQExpBuffer(&buf,
+ ",\n pg_catalog.obj_description(c.oid, 'pg_class') as \"%s\"",
+ gettext_noop("Description"));
appendPQExpBufferStr(&buf,
"\nFROM pg_catalog.pg_class c"
--------------2.20.1--
* Re: nRe: [PATCH v1] Show whether tables are logged in \dt+
@ 2019-04-27 20:38 Fabien COELHO <coelho@cri.ensmp.fr>
parent: David Fetter <david@fetter.org>
0 siblings, 0 replies; 3+ messages in thread
From: Fabien COELHO @ 2019-04-27 20:38 UTC (permalink / raw)
To: David Fetter <david@fetter.org>; +Cc: Rafia Sabih <rafia.pghackers@gmail.com>; pgsql-hackers
Hello David,
Patch applies. There seems to be a compilation issue:
describe.c:5974:1: error: expected declaration or statement at end of
input
}
Also there is an added indentation problem: the size & description stuff
have been moved left but it should still in the verbose case, and a } is
missing after them, which fixes the compilation.
>> Make check fails because of my temp schema was numbered 4 instead of 3, and
>> I'm "fabien" rather than "shackle".
>
> I think the way forward is to test this with TAP rather than the
> fixed-string method.
Ok.
> Checks removed while I figure out a new TAP test.
>> I only have packages down to pg 9.3, so I could not test prior 9.1.
>> By looking at the online documentation, is seems that relistemp
>> appears in pg 8.4, so the corresponding extraction should be guarded
>> by this version. Before that, temporary objects existed but were
>> identified indirectly, possibly because they were stored in a
>> temporary schema. I suggest not to try to address cases prior 8.4.
>
> Done.
After some checking, I think that there is an issue with the version
numbers:
- 9.1 is 90100, not 91000
- 8.4 is 80400, not 84000
Also, it seems that describes builds queries with uppercase keywords, so
probably the new additions should follow that style: case -> CASE (and
also when then else end as…)
--
Fabien.
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2019-04-27 20:38 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-04-27 07:19 nRe: [PATCH v1] Show whether tables are logged in \dt+ Fabien COELHO <coelho@cri.ensmp.fr>
2019-04-27 17:43 ` David Fetter <david@fetter.org>
2019-04-27 20:38 ` Fabien COELHO <coelho@cri.ensmp.fr>
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