proposal - patch: psql - sort_by_size
Hi
I returned to possibility to sort output of \d* and \l by size. There was
more a experiments in this area, but without success. Last patch was
example of over engineering, and now, I try to implement this feature
simply how it is possible. I don't think so we need too complex solution -
if somebody needs specific report, then it is not hard to run psql with
"-E" option, get and modify used query (and use a power of SQL). But
displaying databases objects sorted by size is very common case.
This proposal is based on new psql variable "SORT_BY_SIZE". This variable
will be off by default. The value of this variable is used only in verbose
mode (when the size is displayed - I don't see any benefit sort of size
without showing size). Usage is very simple and implementation too:
\dt -- sorted by schema, name
\dt+ -- still sorted by schema, name
\set SORT_BY_SIZE on
\dt -- sorted by schema, name (size is not calculated and is not visible)
\dt+ -- sorted by size
\dt+ public.* -- sorted by size from schema public
Comments, notes?
Regards
Pavel
Attachments:
psql-sort-by-size.patchtext/x-patch; charset=US-ASCII; name=psql-sort-by-size.patchDownload
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index c6c20de243..14fae1abd3 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3959,6 +3959,17 @@ bar
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><varname>SORT_BY_SIZE</varname></term>
+ <listitem>
+ <para>
+ Setting this variable to <literal>on</literal> causes so results of
+ <literal>\d*</literal> commands will be sorted by size, when size
+ is displayed.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><varname>SQLSTATE</varname></term>
<listitem>
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 97167d2c4b..be149391a1 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -223,6 +223,7 @@ describeTablespaces(const char *pattern, bool verbose)
PQExpBufferData buf;
PGresult *res;
printQueryOpt myopt = pset.popt;
+ const char *sizefunc = NULL;
if (pset.sversion < 80000)
{
@@ -265,9 +266,12 @@ describeTablespaces(const char *pattern, bool verbose)
gettext_noop("Options"));
if (verbose && pset.sversion >= 90200)
+ {
appendPQExpBuffer(&buf,
",\n pg_catalog.pg_size_pretty(pg_catalog.pg_tablespace_size(oid)) AS \"%s\"",
gettext_noop("Size"));
+ sizefunc = "pg_catalog.pg_tablespace_size(oid)";
+ }
if (verbose && pset.sversion >= 80200)
appendPQExpBuffer(&buf,
@@ -281,7 +285,10 @@ describeTablespaces(const char *pattern, bool verbose)
NULL, "spcname", NULL,
NULL);
- appendPQExpBufferStr(&buf, "ORDER BY 1;");
+ if (pset.sort_by_size && sizefunc)
+ appendPQExpBuffer(&buf, "ORDER BY %s DESC;", sizefunc);
+ else
+ appendPQExpBufferStr(&buf, "ORDER BY 1;");
res = PSQLexec(buf.data);
termPQExpBuffer(&buf);
@@ -863,6 +870,7 @@ listAllDbs(const char *pattern, bool verbose)
PGresult *res;
PQExpBufferData buf;
printQueryOpt myopt = pset.popt;
+ const char *sizefunc = NULL;
initPQExpBuffer(&buf);
@@ -882,12 +890,15 @@ listAllDbs(const char *pattern, bool verbose)
appendPQExpBufferStr(&buf, " ");
printACLColumn(&buf, "d.datacl");
if (verbose && pset.sversion >= 80200)
+ {
appendPQExpBuffer(&buf,
",\n CASE WHEN pg_catalog.has_database_privilege(d.datname, 'CONNECT')\n"
" THEN pg_catalog.pg_size_pretty(pg_catalog.pg_database_size(d.datname))\n"
" ELSE 'No Access'\n"
" END as \"%s\"",
gettext_noop("Size"));
+ sizefunc = "pg_catalog.pg_database_size(d.datname)";
+ }
if (verbose && pset.sversion >= 80000)
appendPQExpBuffer(&buf,
",\n t.spcname as \"%s\"",
@@ -906,7 +917,10 @@ listAllDbs(const char *pattern, bool verbose)
processSQLNamePattern(pset.db, &buf, pattern, false, false,
NULL, "d.datname", NULL, NULL);
- appendPQExpBufferStr(&buf, "ORDER BY 1;");
+ if (pset.sort_by_size && sizefunc)
+ appendPQExpBuffer(&buf, "ORDER BY %s DESC;", sizefunc);
+ else
+ appendPQExpBufferStr(&buf, "ORDER BY 1;");
res = PSQLexec(buf.data);
termPQExpBuffer(&buf);
if (!res)
@@ -3628,6 +3642,7 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
bool showMatViews = strchr(tabtypes, 'm') != NULL;
bool showSeq = strchr(tabtypes, 's') != NULL;
bool showForeign = strchr(tabtypes, 'E') != NULL;
+ const char *sizefunc = NULL;
PQExpBufferData buf;
PGresult *res;
@@ -3685,13 +3700,19 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
* 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"));
+ sizefunc = "pg_catalog.pg_table_size(c.oid)";
+ }
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"));
+ sizefunc = "pg_catalog.pg_relation_size(c.oid)";
+ }
appendPQExpBuffer(&buf,
",\n pg_catalog.obj_description(c.oid, 'pg_class') as \"%s\"",
@@ -3744,7 +3765,10 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
"n.nspname", "c.relname", NULL,
"pg_catalog.pg_table_is_visible(c.oid)");
- appendPQExpBufferStr(&buf, "ORDER BY 1,2;");
+ if (pset.sort_by_size && sizefunc)
+ appendPQExpBuffer(&buf, "ORDER BY %s DESC;", sizefunc);
+ else
+ appendPQExpBufferStr(&buf, "ORDER BY 1,2;");
res = PSQLexec(buf.data);
termPQExpBuffer(&buf);
@@ -3920,6 +3944,7 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
" JOIN d ON i.inhparent = d.oid)\n"
" SELECT pg_catalog.pg_size_pretty(sum(pg_catalog.pg_table_size("
"d.oid))) AS tps,\n"
+ " sum(pg_catalog.pg_table_size(d.oid)) AS rps,\n"
" pg_catalog.pg_size_pretty(sum("
"\n CASE WHEN d.level = 1"
" THEN pg_catalog.pg_table_size(d.oid) ELSE 0 END)) AS dps\n"
@@ -3935,6 +3960,7 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
" ELSE 0 END)) AS dps"
",\n pg_catalog.pg_size_pretty(sum("
"pg_catalog.pg_table_size(ppt.relid))) AS tps"
+ ",\n sum(pg_catalog.pg_table_size(ppt.relid)) AS rps"
"\n FROM pg_catalog.pg_partition_tree(c.oid) ppt) s");
}
}
@@ -3967,9 +3993,14 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
"n.nspname", "c.relname", NULL,
"pg_catalog.pg_table_is_visible(c.oid)");
- appendPQExpBuffer(&buf, "ORDER BY \"Schema\", %s%s\"Name\";",
- mixed_output ? "\"Type\" DESC, " : "",
- showNested || pattern ? "\"Parent name\" NULLS FIRST, " : "");
+ if (pset.sort_by_size && verbose)
+ appendPQExpBuffer(&buf, "ORDER BY %s%srps DESC",
+ mixed_output ? "\"Type\" DESC, " : "",
+ showNested || pattern ? "\"Parent name\" NULLS FIRST, " : "");
+ else
+ appendPQExpBuffer(&buf, "ORDER BY \"Schema\", %s%s\"Name\";",
+ mixed_output ? "\"Type\" DESC, " : "",
+ showNested || pattern ? "\"Parent name\" NULLS FIRST, " : "");
res = PSQLexec(buf.data);
termPQExpBuffer(&buf);
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 5fb1baadc5..c4665bca2c 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -341,7 +341,7 @@ helpVariables(unsigned short int pager)
* Windows builds currently print one more line than non-Windows builds.
* Using the larger number is fine.
*/
- output = PageOutput(158, pager ? &(pset.popt.topt) : NULL);
+ output = PageOutput(160, pager ? &(pset.popt.topt) : NULL);
fprintf(output, _("List of specially treated variables\n\n"));
@@ -410,6 +410,8 @@ helpVariables(unsigned short int pager)
" if set, end of line terminates SQL commands (same as -S option)\n"));
fprintf(output, _(" SINGLESTEP\n"
" single-step mode (same as -s option)\n"));
+ fprintf(output, _(" SORT_BY_SIZE\n"
+ " describe reports are sorted by size when size is displayed\n"));
fprintf(output, _(" SQLSTATE\n"
" SQLSTATE of last query, or \"00000\" if no error\n"));
fprintf(output, _(" USER\n"
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 5be5091f0e..dc0033652b 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -127,6 +127,7 @@ typedef struct _psqlSettings
bool quiet;
bool singleline;
bool singlestep;
+ bool sort_by_size;
bool hide_tableam;
int fetch_count;
int histsize;
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index bb9921a894..dc56188248 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -883,6 +883,12 @@ singlestep_hook(const char *newval)
return ParseVariableBool(newval, "SINGLESTEP", &pset.singlestep);
}
+static bool
+sort_by_size_hook(const char *newval)
+{
+ return ParseVariableBool(newval, "SORT_BY_SIZE", &pset.sort_by_size);
+}
+
static char *
fetch_count_substitute_hook(char *newval)
{
@@ -1183,6 +1189,9 @@ EstablishVariableSpace(void)
SetVariableHooks(pset.vars, "SINGLESTEP",
bool_substitute_hook,
singlestep_hook);
+ SetVariableHooks(pset.vars, "SORT_BY_SIZE",
+ bool_substitute_hook,
+ sort_by_size_hook);
SetVariableHooks(pset.vars, "FETCH_COUNT",
fetch_count_substitute_hook,
fetch_count_hook);
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 7dcf342413..edea5fae0d 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3682,7 +3682,7 @@ psql_completion(const char *text, int start, int end)
else if (TailMatchesCS("\\set", MatchAny))
{
if (TailMatchesCS("AUTOCOMMIT|ON_ERROR_STOP|QUIET|"
- "SINGLELINE|SINGLESTEP"))
+ "SINGLELINE|SINGLESTEP|SORT_BY_SIZE"))
COMPLETE_WITH_CS("on", "off");
else if (TailMatchesCS("COMP_KEYWORD_CASE"))
COMPLETE_WITH_CS("lower", "upper",
Hello Pavel,
\set SORT_BY_SIZE on
\dt -- sorted by schema, name (size is not calculated and is not visible)
\dt+ -- sorted by size
Patch applies cleanly, compiles, runs. "make check" ok. doc build ok.
There are no tests. Some infrastructure should be in place so that such
features can be tested, eg so psql-specific TAP tests. ISTM that there was
a patch submitted for that, but I cannot find it:-( Maybe it is combined
with some other patch in the CF.
I agree that the simpler the better for such a feature.
ISTM that the fact that the option is ignored on \dt is a little bit
annoying. It means that \dt and \dt+ would not show their results in the
same order. I understand that the point is to avoid the cost of computing
the sizes, but if the user asked for it, should it be done anyway?
I'm wondering whether it would make sense to have a slightly more generic
interface allowing for more values, eg:
\set DESCRIPTION_SORT "name"
\set DESCRIPTION_SORT "size"
Well, possibly this is a bad idea, so it is really a question.
+ Setting this variable to <literal>on</literal> causes so results of
+ <literal>\d*</literal> commands will be sorted by size, when size
+ is displayed.Maybe the simpler: "Setting this variable on sorts \d* outputs by size,
when size is displayed."
ISTM that the documentation is more generic than reality. Does it work
with \db+? It seems to work with \dm+.
On equality, ISTM it it should sort by name as a secondary criterion.
I tested a few cases, although not partitioned tables.
--
Fabien.
so 29. 6. 2019 v 9:32 odesílatel Fabien COELHO <coelho@cri.ensmp.fr> napsal:
Hello Pavel,
\set SORT_BY_SIZE on
\dt -- sorted by schema, name (size is not calculated and is not visible)
\dt+ -- sorted by sizePatch applies cleanly, compiles, runs. "make check" ok. doc build ok.
There are no tests. Some infrastructure should be in place so that such
features can be tested, eg so psql-specific TAP tests. ISTM that there was
a patch submitted for that, but I cannot find it:-( Maybe it is combined
with some other patch in the CF.
It is not possible - the size of relations is not stable (can be different
on some platforms), and because showing the size is base of this patch, I
cannot to write tests. Maybe only only set/unset of variable.
I agree that the simpler the better for such a feature.
ISTM that the fact that the option is ignored on \dt is a little bit
annoying. It means that \dt and \dt+ would not show their results in the
same order. I understand that the point is to avoid the cost of computing
the sizes, but if the user asked for it, should it be done anyway?
It was one objection against some previous patches. In this moment I don't
see any wrong on different order between \dt and \dt+. When column "size"
will be displayed, then ordering of report will be clean.
I am not strongly against this - implementation of support SORT_BY_SIZE for
non verbose mode is +/- few lines more. But now (and it is just my opinion
and filing, nothing more), I think so sorting reports by invisible columns
can be messy. But if somebody will have strong different option on this
point, I am able to accept it. Both variants can have some sense, and some
benefits - both variants are consistent with some rules (but cannot be
together).
I'm wondering whether it would make sense to have a slightly more generic
interface allowing for more values, eg:\set DESCRIPTION_SORT "name"
\set DESCRIPTION_SORT "size"Well, possibly this is a bad idea, so it is really a question.
We was at this point already :). If you introduce this, then you have to
support combinations schema_name, name_schema, size, schema_size, ...
My goal is implementation of most common missing alternative into psql -
but I would not to do too generic implementation - it needs more complex
design (and UI), and I don't think so people use it. SORT_BY_SIZE (on/off)
looks simply, and because (if will not be changed) it has not impact on non
verbose mode, then it can be active permanently (and if not, it is not
mental hard work to set it).
I think so more generic solution needs interactive UI. Now I working on
vertical cursor support for pspg https://github.com/okbob/pspg. Next step
will be sort by column under vertical cursor. So, I hope, it can be good
enough for simply sorting by any column of report (but to be user friendly,
it needs interactive UI). Because not everywhere is pspg installed, I would
to push some simple solution (I prefer simplicity against generic) to psql.
+ Setting this variable to <literal>on</literal> causes so results of + <literal>\d*</literal> commands will be sorted by size, when size + is displayed.Maybe the simpler: "Setting this variable on sorts \d* outputs by size,
when size is displayed."ISTM that the documentation is more generic than reality. Does it work
with \db+? It seems to work with \dm+.On equality, ISTM it it should sort by name as a secondary criterion.
I tested a few cases, although not partitioned tables.
Thank you - I support now relations (tables, indexes, ), databases, and
tablespaces. The column size is displayed for data types report, but I am
not sure about any benefit in this case.
Regards
Pavel
Show quoted text
--
Fabien.
Hi
so 29. 6. 2019 v 10:19 odesílatel Pavel Stehule <pavel.stehule@gmail.com>
napsal:
so 29. 6. 2019 v 9:32 odesílatel Fabien COELHO <coelho@cri.ensmp.fr>
napsal:Hello Pavel,
\set SORT_BY_SIZE on
\dt -- sorted by schema, name (size is not calculated and is notvisible)
\dt+ -- sorted by size
Patch applies cleanly, compiles, runs. "make check" ok. doc build ok.
There are no tests. Some infrastructure should be in place so that such
features can be tested, eg so psql-specific TAP tests. ISTM that there
was
a patch submitted for that, but I cannot find it:-( Maybe it is combined
with some other patch in the CF.It is not possible - the size of relations is not stable (can be different
on some platforms), and because showing the size is base of this patch, I
cannot to write tests. Maybe only only set/unset of variable.I agree that the simpler the better for such a feature.
ISTM that the fact that the option is ignored on \dt is a little bit
annoying. It means that \dt and \dt+ would not show their results in the
same order. I understand that the point is to avoid the cost of computing
the sizes, but if the user asked for it, should it be done anyway?It was one objection against some previous patches. In this moment I don't
see any wrong on different order between \dt and \dt+. When column "size"
will be displayed, then ordering of report will be clean.I am not strongly against this - implementation of support SORT_BY_SIZE
for non verbose mode is +/- few lines more. But now (and it is just my
opinion and filing, nothing more), I think so sorting reports by invisible
columns can be messy. But if somebody will have strong different option on
this point, I am able to accept it. Both variants can have some sense, and
some benefits - both variants are consistent with some rules (but cannot be
together).I'm wondering whether it would make sense to have a slightly more generic
interface allowing for more values, eg:\set DESCRIPTION_SORT "name"
\set DESCRIPTION_SORT "size"Well, possibly this is a bad idea, so it is really a question.
We was at this point already :). If you introduce this, then you have to
support combinations schema_name, name_schema, size, schema_size, ...My goal is implementation of most common missing alternative into psql -
but I would not to do too generic implementation - it needs more complex
design (and UI), and I don't think so people use it. SORT_BY_SIZE (on/off)
looks simply, and because (if will not be changed) it has not impact on non
verbose mode, then it can be active permanently (and if not, it is not
mental hard work to set it).I think so more generic solution needs interactive UI. Now I working on
vertical cursor support for pspg https://github.com/okbob/pspg. Next step
will be sort by column under vertical cursor. So, I hope, it can be good
enough for simply sorting by any column of report (but to be user friendly,
it needs interactive UI). Because not everywhere is pspg installed, I would
to push some simple solution (I prefer simplicity against generic) to psql.+ Setting this variable to <literal>on</literal> causes so results of + <literal>\d*</literal> commands will be sorted by size, when size + is displayed.Maybe the simpler: "Setting this variable on sorts \d* outputs by size,
when size is displayed."
I used this text in today patch
Regards
Pavel
Show quoted text
ISTM that the documentation is more generic than reality. Does it work
with \db+? It seems to work with \dm+.On equality, ISTM it it should sort by name as a secondary criterion.
I tested a few cases, although not partitioned tables.
Thank you - I support now relations (tables, indexes, ), databases, and
tablespaces. The column size is displayed for data types report, but I am
not sure about any benefit in this case.Regards
Pavel
--
Fabien.
Attachments:
psql-sort-by-size-20190630.patchtext/x-patch; charset=US-ASCII; name=psql-sort-by-size-20190630.patchDownload
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index c6c20de243..b04e93c1f2 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3959,6 +3959,17 @@ bar
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><varname>SORT_BY_SIZE</varname></term>
+ <listitem>
+ <para>
+ Setting this variable to <literal>on</literal>, sorts
+ <literal>\d*+</literal>, <literal>\db+</literal>, <literal>\l+</literal>
+ and <literal>\dP*+</literal> outputs by size (when size is displayed).
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><varname>SQLSTATE</varname></term>
<listitem>
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 97167d2c4b..be149391a1 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -223,6 +223,7 @@ describeTablespaces(const char *pattern, bool verbose)
PQExpBufferData buf;
PGresult *res;
printQueryOpt myopt = pset.popt;
+ const char *sizefunc = NULL;
if (pset.sversion < 80000)
{
@@ -265,9 +266,12 @@ describeTablespaces(const char *pattern, bool verbose)
gettext_noop("Options"));
if (verbose && pset.sversion >= 90200)
+ {
appendPQExpBuffer(&buf,
",\n pg_catalog.pg_size_pretty(pg_catalog.pg_tablespace_size(oid)) AS \"%s\"",
gettext_noop("Size"));
+ sizefunc = "pg_catalog.pg_tablespace_size(oid)";
+ }
if (verbose && pset.sversion >= 80200)
appendPQExpBuffer(&buf,
@@ -281,7 +285,10 @@ describeTablespaces(const char *pattern, bool verbose)
NULL, "spcname", NULL,
NULL);
- appendPQExpBufferStr(&buf, "ORDER BY 1;");
+ if (pset.sort_by_size && sizefunc)
+ appendPQExpBuffer(&buf, "ORDER BY %s DESC;", sizefunc);
+ else
+ appendPQExpBufferStr(&buf, "ORDER BY 1;");
res = PSQLexec(buf.data);
termPQExpBuffer(&buf);
@@ -863,6 +870,7 @@ listAllDbs(const char *pattern, bool verbose)
PGresult *res;
PQExpBufferData buf;
printQueryOpt myopt = pset.popt;
+ const char *sizefunc = NULL;
initPQExpBuffer(&buf);
@@ -882,12 +890,15 @@ listAllDbs(const char *pattern, bool verbose)
appendPQExpBufferStr(&buf, " ");
printACLColumn(&buf, "d.datacl");
if (verbose && pset.sversion >= 80200)
+ {
appendPQExpBuffer(&buf,
",\n CASE WHEN pg_catalog.has_database_privilege(d.datname, 'CONNECT')\n"
" THEN pg_catalog.pg_size_pretty(pg_catalog.pg_database_size(d.datname))\n"
" ELSE 'No Access'\n"
" END as \"%s\"",
gettext_noop("Size"));
+ sizefunc = "pg_catalog.pg_database_size(d.datname)";
+ }
if (verbose && pset.sversion >= 80000)
appendPQExpBuffer(&buf,
",\n t.spcname as \"%s\"",
@@ -906,7 +917,10 @@ listAllDbs(const char *pattern, bool verbose)
processSQLNamePattern(pset.db, &buf, pattern, false, false,
NULL, "d.datname", NULL, NULL);
- appendPQExpBufferStr(&buf, "ORDER BY 1;");
+ if (pset.sort_by_size && sizefunc)
+ appendPQExpBuffer(&buf, "ORDER BY %s DESC;", sizefunc);
+ else
+ appendPQExpBufferStr(&buf, "ORDER BY 1;");
res = PSQLexec(buf.data);
termPQExpBuffer(&buf);
if (!res)
@@ -3628,6 +3642,7 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
bool showMatViews = strchr(tabtypes, 'm') != NULL;
bool showSeq = strchr(tabtypes, 's') != NULL;
bool showForeign = strchr(tabtypes, 'E') != NULL;
+ const char *sizefunc = NULL;
PQExpBufferData buf;
PGresult *res;
@@ -3685,13 +3700,19 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
* 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"));
+ sizefunc = "pg_catalog.pg_table_size(c.oid)";
+ }
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"));
+ sizefunc = "pg_catalog.pg_relation_size(c.oid)";
+ }
appendPQExpBuffer(&buf,
",\n pg_catalog.obj_description(c.oid, 'pg_class') as \"%s\"",
@@ -3744,7 +3765,10 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
"n.nspname", "c.relname", NULL,
"pg_catalog.pg_table_is_visible(c.oid)");
- appendPQExpBufferStr(&buf, "ORDER BY 1,2;");
+ if (pset.sort_by_size && sizefunc)
+ appendPQExpBuffer(&buf, "ORDER BY %s DESC;", sizefunc);
+ else
+ appendPQExpBufferStr(&buf, "ORDER BY 1,2;");
res = PSQLexec(buf.data);
termPQExpBuffer(&buf);
@@ -3920,6 +3944,7 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
" JOIN d ON i.inhparent = d.oid)\n"
" SELECT pg_catalog.pg_size_pretty(sum(pg_catalog.pg_table_size("
"d.oid))) AS tps,\n"
+ " sum(pg_catalog.pg_table_size(d.oid)) AS rps,\n"
" pg_catalog.pg_size_pretty(sum("
"\n CASE WHEN d.level = 1"
" THEN pg_catalog.pg_table_size(d.oid) ELSE 0 END)) AS dps\n"
@@ -3935,6 +3960,7 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
" ELSE 0 END)) AS dps"
",\n pg_catalog.pg_size_pretty(sum("
"pg_catalog.pg_table_size(ppt.relid))) AS tps"
+ ",\n sum(pg_catalog.pg_table_size(ppt.relid)) AS rps"
"\n FROM pg_catalog.pg_partition_tree(c.oid) ppt) s");
}
}
@@ -3967,9 +3993,14 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
"n.nspname", "c.relname", NULL,
"pg_catalog.pg_table_is_visible(c.oid)");
- appendPQExpBuffer(&buf, "ORDER BY \"Schema\", %s%s\"Name\";",
- mixed_output ? "\"Type\" DESC, " : "",
- showNested || pattern ? "\"Parent name\" NULLS FIRST, " : "");
+ if (pset.sort_by_size && verbose)
+ appendPQExpBuffer(&buf, "ORDER BY %s%srps DESC",
+ mixed_output ? "\"Type\" DESC, " : "",
+ showNested || pattern ? "\"Parent name\" NULLS FIRST, " : "");
+ else
+ appendPQExpBuffer(&buf, "ORDER BY \"Schema\", %s%s\"Name\";",
+ mixed_output ? "\"Type\" DESC, " : "",
+ showNested || pattern ? "\"Parent name\" NULLS FIRST, " : "");
res = PSQLexec(buf.data);
termPQExpBuffer(&buf);
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 5fb1baadc5..f9b4e85cbf 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -341,7 +341,7 @@ helpVariables(unsigned short int pager)
* Windows builds currently print one more line than non-Windows builds.
* Using the larger number is fine.
*/
- output = PageOutput(158, pager ? &(pset.popt.topt) : NULL);
+ output = PageOutput(160, pager ? &(pset.popt.topt) : NULL);
fprintf(output, _("List of specially treated variables\n\n"));
@@ -410,6 +410,8 @@ helpVariables(unsigned short int pager)
" if set, end of line terminates SQL commands (same as -S option)\n"));
fprintf(output, _(" SINGLESTEP\n"
" single-step mode (same as -s option)\n"));
+ fprintf(output, _(" SORT_BY_SIZE\n"
+ " if set, outputs of \\d*+, \\db+, \\l+, \\dP*+ are sorted by size\n"));
fprintf(output, _(" SQLSTATE\n"
" SQLSTATE of last query, or \"00000\" if no error\n"));
fprintf(output, _(" USER\n"
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 5be5091f0e..dc0033652b 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -127,6 +127,7 @@ typedef struct _psqlSettings
bool quiet;
bool singleline;
bool singlestep;
+ bool sort_by_size;
bool hide_tableam;
int fetch_count;
int histsize;
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index bb9921a894..dc56188248 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -883,6 +883,12 @@ singlestep_hook(const char *newval)
return ParseVariableBool(newval, "SINGLESTEP", &pset.singlestep);
}
+static bool
+sort_by_size_hook(const char *newval)
+{
+ return ParseVariableBool(newval, "SORT_BY_SIZE", &pset.sort_by_size);
+}
+
static char *
fetch_count_substitute_hook(char *newval)
{
@@ -1183,6 +1189,9 @@ EstablishVariableSpace(void)
SetVariableHooks(pset.vars, "SINGLESTEP",
bool_substitute_hook,
singlestep_hook);
+ SetVariableHooks(pset.vars, "SORT_BY_SIZE",
+ bool_substitute_hook,
+ sort_by_size_hook);
SetVariableHooks(pset.vars, "FETCH_COUNT",
fetch_count_substitute_hook,
fetch_count_hook);
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 7dcf342413..edea5fae0d 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3682,7 +3682,7 @@ psql_completion(const char *text, int start, int end)
else if (TailMatchesCS("\\set", MatchAny))
{
if (TailMatchesCS("AUTOCOMMIT|ON_ERROR_STOP|QUIET|"
- "SINGLELINE|SINGLESTEP"))
+ "SINGLELINE|SINGLESTEP|SORT_BY_SIZE"))
COMPLETE_WITH_CS("on", "off");
else if (TailMatchesCS("COMP_KEYWORD_CASE"))
COMPLETE_WITH_CS("lower", "upper",
On Sun, Jun 30, 2019 at 8:48 PM Pavel Stehule <pavel.stehule@gmail.com> wrote:
I used this text in today patch
Hi Pavel,
Could you please post a rebased patch?
Thanks,
--
Thomas Munro
https://enterprisedb.com
Hi
po 8. 7. 2019 v 6:12 odesílatel Thomas Munro <thomas.munro@gmail.com>
napsal:
On Sun, Jun 30, 2019 at 8:48 PM Pavel Stehule <pavel.stehule@gmail.com>
wrote:I used this text in today patch
Hi Pavel,
Could you please post a rebased patch?
rebased patch attached
Regards
Pavel
Show quoted text
Thanks,
--
Thomas Munro
https://enterprisedb.com
Attachments:
psql-sort-by-size-20190708.patchtext/x-patch; charset=US-ASCII; name=psql-sort-by-size-20190708.patchDownload
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 7789fc6177..96fc4f489a 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3973,6 +3973,17 @@ bar
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><varname>SORT_BY_SIZE</varname></term>
+ <listitem>
+ <para>
+ Setting this variable to <literal>on</literal>, sorts
+ <literal>\d*+</literal>, <literal>\db+</literal>, <literal>\l+</literal>
+ and <literal>\dP*+</literal> outputs by size (when size is displayed).
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><varname>SQLSTATE</varname></term>
<listitem>
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 8b4cd53631..ae79cbb312 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -223,6 +223,7 @@ describeTablespaces(const char *pattern, bool verbose)
PQExpBufferData buf;
PGresult *res;
printQueryOpt myopt = pset.popt;
+ const char *sizefunc = NULL;
if (pset.sversion < 80000)
{
@@ -265,9 +266,12 @@ describeTablespaces(const char *pattern, bool verbose)
gettext_noop("Options"));
if (verbose && pset.sversion >= 90200)
+ {
appendPQExpBuffer(&buf,
",\n pg_catalog.pg_size_pretty(pg_catalog.pg_tablespace_size(oid)) AS \"%s\"",
gettext_noop("Size"));
+ sizefunc = "pg_catalog.pg_tablespace_size(oid)";
+ }
if (verbose && pset.sversion >= 80200)
appendPQExpBuffer(&buf,
@@ -281,7 +285,10 @@ describeTablespaces(const char *pattern, bool verbose)
NULL, "spcname", NULL,
NULL);
- appendPQExpBufferStr(&buf, "ORDER BY 1;");
+ if (pset.sort_by_size && sizefunc)
+ appendPQExpBuffer(&buf, "ORDER BY %s DESC;", sizefunc);
+ else
+ appendPQExpBufferStr(&buf, "ORDER BY 1;");
res = PSQLexec(buf.data);
termPQExpBuffer(&buf);
@@ -863,6 +870,7 @@ listAllDbs(const char *pattern, bool verbose)
PGresult *res;
PQExpBufferData buf;
printQueryOpt myopt = pset.popt;
+ const char *sizefunc = NULL;
initPQExpBuffer(&buf);
@@ -882,12 +890,15 @@ listAllDbs(const char *pattern, bool verbose)
appendPQExpBufferStr(&buf, " ");
printACLColumn(&buf, "d.datacl");
if (verbose && pset.sversion >= 80200)
+ {
appendPQExpBuffer(&buf,
",\n CASE WHEN pg_catalog.has_database_privilege(d.datname, 'CONNECT')\n"
" THEN pg_catalog.pg_size_pretty(pg_catalog.pg_database_size(d.datname))\n"
" ELSE 'No Access'\n"
" END as \"%s\"",
gettext_noop("Size"));
+ sizefunc = "pg_catalog.pg_database_size(d.datname)";
+ }
if (verbose && pset.sversion >= 80000)
appendPQExpBuffer(&buf,
",\n t.spcname as \"%s\"",
@@ -906,7 +917,10 @@ listAllDbs(const char *pattern, bool verbose)
processSQLNamePattern(pset.db, &buf, pattern, false, false,
NULL, "d.datname", NULL, NULL);
- appendPQExpBufferStr(&buf, "ORDER BY 1;");
+ if (pset.sort_by_size && sizefunc)
+ appendPQExpBuffer(&buf, "ORDER BY %s DESC;", sizefunc);
+ else
+ appendPQExpBufferStr(&buf, "ORDER BY 1;");
res = PSQLexec(buf.data);
termPQExpBuffer(&buf);
if (!res)
@@ -3628,6 +3642,7 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
bool showMatViews = strchr(tabtypes, 'm') != NULL;
bool showSeq = strchr(tabtypes, 's') != NULL;
bool showForeign = strchr(tabtypes, 'E') != NULL;
+ const char *sizefunc = NULL;
PQExpBufferData buf;
PGresult *res;
@@ -3711,13 +3726,19 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
* 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"));
+ sizefunc = "pg_catalog.pg_table_size(c.oid)";
+ }
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"));
+ sizefunc = "pg_catalog.pg_relation_size(c.oid)";
+ }
appendPQExpBuffer(&buf,
",\n pg_catalog.obj_description(c.oid, 'pg_class') as \"%s\"",
@@ -3770,7 +3791,10 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
"n.nspname", "c.relname", NULL,
"pg_catalog.pg_table_is_visible(c.oid)");
- appendPQExpBufferStr(&buf, "ORDER BY 1,2;");
+ if (pset.sort_by_size && sizefunc)
+ appendPQExpBuffer(&buf, "ORDER BY %s DESC;", sizefunc);
+ else
+ appendPQExpBufferStr(&buf, "ORDER BY 1,2;");
res = PSQLexec(buf.data);
termPQExpBuffer(&buf);
@@ -3946,6 +3970,7 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
" JOIN d ON i.inhparent = d.oid)\n"
" SELECT pg_catalog.pg_size_pretty(sum(pg_catalog.pg_table_size("
"d.oid))) AS tps,\n"
+ " sum(pg_catalog.pg_table_size(d.oid)) AS rps,\n"
" pg_catalog.pg_size_pretty(sum("
"\n CASE WHEN d.level = 1"
" THEN pg_catalog.pg_table_size(d.oid) ELSE 0 END)) AS dps\n"
@@ -3961,6 +3986,7 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
" ELSE 0 END)) AS dps"
",\n pg_catalog.pg_size_pretty(sum("
"pg_catalog.pg_table_size(ppt.relid))) AS tps"
+ ",\n sum(pg_catalog.pg_table_size(ppt.relid)) AS rps"
"\n FROM pg_catalog.pg_partition_tree(c.oid) ppt) s");
}
}
@@ -3993,9 +4019,14 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
"n.nspname", "c.relname", NULL,
"pg_catalog.pg_table_is_visible(c.oid)");
- appendPQExpBuffer(&buf, "ORDER BY \"Schema\", %s%s\"Name\";",
- mixed_output ? "\"Type\" DESC, " : "",
- showNested || pattern ? "\"Parent name\" NULLS FIRST, " : "");
+ if (pset.sort_by_size && verbose)
+ appendPQExpBuffer(&buf, "ORDER BY %s%srps DESC",
+ mixed_output ? "\"Type\" DESC, " : "",
+ showNested || pattern ? "\"Parent name\" NULLS FIRST, " : "");
+ else
+ appendPQExpBuffer(&buf, "ORDER BY \"Schema\", %s%s\"Name\";",
+ mixed_output ? "\"Type\" DESC, " : "",
+ showNested || pattern ? "\"Parent name\" NULLS FIRST, " : "");
res = PSQLexec(buf.data);
termPQExpBuffer(&buf);
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index d9b982d3a0..2037ba352f 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -342,7 +342,7 @@ helpVariables(unsigned short int pager)
* Windows builds currently print one more line than non-Windows builds.
* Using the larger number is fine.
*/
- output = PageOutput(158, pager ? &(pset.popt.topt) : NULL);
+ output = PageOutput(160, pager ? &(pset.popt.topt) : NULL);
fprintf(output, _("List of specially treated variables\n\n"));
@@ -411,6 +411,8 @@ helpVariables(unsigned short int pager)
" if set, end of line terminates SQL commands (same as -S option)\n"));
fprintf(output, _(" SINGLESTEP\n"
" single-step mode (same as -s option)\n"));
+ fprintf(output, _(" SORT_BY_SIZE\n"
+ " if set, outputs of \\d*+, \\db+, \\l+, \\dP*+ are sorted by size\n"));
fprintf(output, _(" SQLSTATE\n"
" SQLSTATE of last query, or \"00000\" if no error\n"));
fprintf(output, _(" USER\n"
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 5be5091f0e..dc0033652b 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -127,6 +127,7 @@ typedef struct _psqlSettings
bool quiet;
bool singleline;
bool singlestep;
+ bool sort_by_size;
bool hide_tableam;
int fetch_count;
int histsize;
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 4730c73396..974c965875 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -884,6 +884,12 @@ singlestep_hook(const char *newval)
return ParseVariableBool(newval, "SINGLESTEP", &pset.singlestep);
}
+static bool
+sort_by_size_hook(const char *newval)
+{
+ return ParseVariableBool(newval, "SORT_BY_SIZE", &pset.sort_by_size);
+}
+
static char *
fetch_count_substitute_hook(char *newval)
{
@@ -1184,6 +1190,9 @@ EstablishVariableSpace(void)
SetVariableHooks(pset.vars, "SINGLESTEP",
bool_substitute_hook,
singlestep_hook);
+ SetVariableHooks(pset.vars, "SORT_BY_SIZE",
+ bool_substitute_hook,
+ sort_by_size_hook);
SetVariableHooks(pset.vars, "FETCH_COUNT",
fetch_count_substitute_hook,
fetch_count_hook);
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 9009b05e12..66d34be43b 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3698,7 +3698,7 @@ psql_completion(const char *text, int start, int end)
else if (TailMatchesCS("\\set", MatchAny))
{
if (TailMatchesCS("AUTOCOMMIT|ON_ERROR_STOP|QUIET|"
- "SINGLELINE|SINGLESTEP"))
+ "SINGLELINE|SINGLESTEP|SORT_BY_SIZE"))
COMPLETE_WITH_CS("on", "off");
else if (TailMatchesCS("COMP_KEYWORD_CASE"))
COMPLETE_WITH_CS("lower", "upper",
Hello Pavel,
rebased patch attached
I prefer patches with a number rather than a date, if possible. For one
thing, there may be several updates in one day.
About this version (20180708, probably v3): applies cleanly, compiles,
make check ok, doc build ok. No tests.
It works for me on a few manual tests against a 11.4 server.
Documentation: if you say "\d*+", then it already applies to \db+ and
\dP+, so why listing them? Otherwise, state all commands or make it work
on all commands that have a size?
About the text:
- remove , before "sorts"
- ... outputs by decreasing size, when size is displayed.
- add: When size is not displayed, the output is sorted by names.
I still think that the object name should be kept as a secondary sort
criterion, in case of size equality, so that the output is deterministic.
Having plenty of objects of the same size out of alphabetical order looks
very strange.
I still do not like much the boolean approach. I understand that the name
approach has been rejected, and I can understand why.
I've been thinking about another more generic interface, that I'm putting
here for discussion, I do not claim that it is a good idea. Probably could
fall under "over engineering", but it might not be much harder to
implement, and it solves a few potential problems.
The idea is to add an option to \d commands, such as "\echo -n":
\dt+ [-o 1d,2a] ...
meaning do the \dt+, order by column 1 descending, column 2 ascending.
With this there would be no need for a special variable nor other
extensions to specify some ordering, whatever the user wishes.
Maybe it could be "\dt+ [-o '1 DESC, 2 ASC'] ..." so that the string
is roughly used as an ORDER BY specification by the query, but it would be
longer to specify.
It also solves the issue that if someone wants another sorting order we
would end with competing boolean variables such as SORT_BY_SIZE,
SORT_BY_TYPE, SORT_BY_SCHEMA, which would be pretty unpractical. The
boolean approach works for *one* sorting extension and breaks at the next
extension.
Also, the boolean does not say that it is a descending order. I could be
interested in looking at the small tables.
Another benefit for me is that I do not like much variables with side
effects, whereas with an explicit syntax there would be no such thing, the
user has what was asked for. Ok, psql is full of them, but I cannot say I
like it for that.
The approach could be extended to specify a limit, eg \dt -l 10 would
add a LIMIT 10 on the query.
Also, the implementation could be high enough so that the description
handlers would not have to deal with it individually, it could return
the query which would then be completed with SORT/LIMIT clauses before
being executed, possibly with a default order if none is specified.
--
Fabien.
pá 12. 7. 2019 v 15:10 odesílatel Fabien COELHO <coelho@cri.ensmp.fr>
napsal:
Hello Pavel,
rebased patch attached
I prefer patches with a number rather than a date, if possible. For one
thing, there may be several updates in one day.About this version (20180708, probably v3): applies cleanly, compiles,
make check ok, doc build ok. No tests.It works for me on a few manual tests against a 11.4 server.
Documentation: if you say "\d*+", then it already applies to \db+ and
\dP+, so why listing them? Otherwise, state all commands or make it work
on all commands that have a size?
\dT+ show sizes too, and there is a mix of values "1, 4, 8, 12, 24, var". I
don't think so sort by size there has sense
About the text:
- remove , before "sorts"
- ... outputs by decreasing size, when size is displayed.
- add: When size is not displayed, the output is sorted by names.
ok
I still think that the object name should be kept as a secondary sort
criterion, in case of size equality, so that the output is deterministic.
Having plenty of objects of the same size out of alphabetical order looks
very strange.I still do not like much the boolean approach. I understand that the name
approach has been rejected, and I can understand why.I've been thinking about another more generic interface, that I'm putting
here for discussion, I do not claim that it is a good idea. Probably could
fall under "over engineering", but it might not be much harder to
implement, and it solves a few potential problems.The idea is to add an option to \d commands, such as "\echo -n":
\dt+ [-o 1d,2a] ...
meaning do the \dt+, order by column 1 descending, column 2 ascending.
With this there would be no need for a special variable nor other
extensions to specify some ordering, whatever the user wishes.Maybe it could be "\dt+ [-o '1 DESC, 2 ASC'] ..." so that the string
is roughly used as an ORDER BY specification by the query, but it would be
longer to specify.
I have two objections - although I think so this functionality can coexists
with functionality implemented by this patch
1. You cannot use column number for sort by size, because this value is
prettified (use pg_size_pretty).
2. Because @1, then there is not simple solution for sort by size
3. This extension should be generic, and then it will be much bigger patch
It also solves the issue that if someone wants another sorting order we
would end with competing boolean variables such as SORT_BY_SIZE,
SORT_BY_TYPE, SORT_BY_SCHEMA, which would be pretty unpractical. The
boolean approach works for *one* sorting extension and breaks at the next
extension.Also, the boolean does not say that it is a descending order. I could be
interested in looking at the small tables.Another benefit for me is that I do not like much variables with side
effects, whereas with an explicit syntax there would be no such thing, the
user has what was asked for. Ok, psql is full of them, but I cannot say I
like it for that.The approach could be extended to specify a limit, eg \dt -l 10 would
add a LIMIT 10 on the query.
It is common problem - when you do some repeated task, then you want to do
quickly. But sometimes you would to do some specialized task, and then you
should to overwrite default setting easy.
Good system should to support both. But commands that allows
parametrization can be hard for learning, hard for use. There are lot of
users of "vim" or "emacs", but most users prefers "notepad".
All is about searching some compromise.
Also, the implementation could be high enough so that the description
handlers would not have to deal with it individually, it could return
the query which would then be completed with SORT/LIMIT clauses before
being executed, possibly with a default order if none is specified.
I don't think so your proposal is bad, and it is not in conflict with this
patch, but it
a) doesn't solve SORT BY SIZE problem
b) requires modification of parser of any related \command - so it will be
bigger and massive patch.
In this moment I prefer my simple implementation still. My patch is related
just for few describe commands. Your proposal should be really generic
(there is not a reason limit it just for reports with size)
Simple boolean design doesn't block any enhancing of future. The effect of
SORT_BY_SIZE variable can be overwritten by some specialized future option
used inside \command.
Regards
Pavel
Show quoted text
--
Fabien.
Hi
pá 12. 7. 2019 v 15:10 odesílatel Fabien COELHO <coelho@cri.ensmp.fr>
napsal:
Hello Pavel,
rebased patch attached
I prefer patches with a number rather than a date, if possible. For one
thing, there may be several updates in one day.About this version (20180708, probably v3): applies cleanly, compiles,
make check ok, doc build ok. No tests.
attached version 4
It works for me on a few manual tests against a 11.4 server.
Documentation: if you say "\d*+", then it already applies to \db+ and
\dP+, so why listing them? Otherwise, state all commands or make it work
on all commands that have a size?About the text:
- remove , before "sorts"
- ... outputs by decreasing size, when size is displayed.
- add: When size is not displayed, the output is sorted by names.
fixed
I still think that the object name should be kept as a secondary sort
criterion, in case of size equality, so that the output is deterministic.
Having plenty of objects of the same size out of alphabetical order looks
very strange.
fixed
Regards
Pavel
Show quoted text
I still do not like much the boolean approach. I understand that the name
approach has been rejected, and I can understand why.I've been thinking about another more generic interface, that I'm putting
here for discussion, I do not claim that it is a good idea. Probably could
fall under "over engineering", but it might not be much harder to
implement, and it solves a few potential problems.The idea is to add an option to \d commands, such as "\echo -n":
\dt+ [-o 1d,2a] ...
meaning do the \dt+, order by column 1 descending, column 2 ascending.
With this there would be no need for a special variable nor other
extensions to specify some ordering, whatever the user wishes.Maybe it could be "\dt+ [-o '1 DESC, 2 ASC'] ..." so that the string
is roughly used as an ORDER BY specification by the query, but it would be
longer to specify.It also solves the issue that if someone wants another sorting order we
would end with competing boolean variables such as SORT_BY_SIZE,
SORT_BY_TYPE, SORT_BY_SCHEMA, which would be pretty unpractical. The
boolean approach works for *one* sorting extension and breaks at the next
extension.Also, the boolean does not say that it is a descending order. I could be
interested in looking at the small tables.Another benefit for me is that I do not like much variables with side
effects, whereas with an explicit syntax there would be no such thing, the
user has what was asked for. Ok, psql is full of them, but I cannot say I
like it for that.The approach could be extended to specify a limit, eg \dt -l 10 would
add a LIMIT 10 on the query.Also, the implementation could be high enough so that the description
handlers would not have to deal with it individually, it could return
the query which would then be completed with SORT/LIMIT clauses before
being executed, possibly with a default order if none is specified.--
Fabien.
Attachments:
psql-sort-by-size-4.patchtext/x-patch; charset=US-ASCII; name=psql-sort-by-size-4.patchDownload
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 7789fc6177..40aeb8cef0 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3973,6 +3973,17 @@ bar
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><varname>SORT_BY_SIZE</varname></term>
+ <listitem>
+ <para>
+ Sorts <literal>\d[bimtv]+</literal> and <literal>\l+</literal>
+ outputs by decreasing size (when size is displayed). When size
+ is not displayed, then output is sorted by name.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><varname>SQLSTATE</varname></term>
<listitem>
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 8b4cd53631..9a7f02607c 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -223,6 +223,7 @@ describeTablespaces(const char *pattern, bool verbose)
PQExpBufferData buf;
PGresult *res;
printQueryOpt myopt = pset.popt;
+ const char *sizefunc = NULL;
if (pset.sversion < 80000)
{
@@ -265,9 +266,12 @@ describeTablespaces(const char *pattern, bool verbose)
gettext_noop("Options"));
if (verbose && pset.sversion >= 90200)
+ {
appendPQExpBuffer(&buf,
",\n pg_catalog.pg_size_pretty(pg_catalog.pg_tablespace_size(oid)) AS \"%s\"",
gettext_noop("Size"));
+ sizefunc = "pg_catalog.pg_tablespace_size(oid)";
+ }
if (verbose && pset.sversion >= 80200)
appendPQExpBuffer(&buf,
@@ -281,7 +285,10 @@ describeTablespaces(const char *pattern, bool verbose)
NULL, "spcname", NULL,
NULL);
- appendPQExpBufferStr(&buf, "ORDER BY 1;");
+ if (pset.sort_by_size && sizefunc)
+ appendPQExpBuffer(&buf, "ORDER BY %s DESC, 1;", sizefunc);
+ else
+ appendPQExpBufferStr(&buf, "ORDER BY 1;");
res = PSQLexec(buf.data);
termPQExpBuffer(&buf);
@@ -863,6 +870,7 @@ listAllDbs(const char *pattern, bool verbose)
PGresult *res;
PQExpBufferData buf;
printQueryOpt myopt = pset.popt;
+ const char *sizefunc = NULL;
initPQExpBuffer(&buf);
@@ -882,12 +890,15 @@ listAllDbs(const char *pattern, bool verbose)
appendPQExpBufferStr(&buf, " ");
printACLColumn(&buf, "d.datacl");
if (verbose && pset.sversion >= 80200)
+ {
appendPQExpBuffer(&buf,
",\n CASE WHEN pg_catalog.has_database_privilege(d.datname, 'CONNECT')\n"
" THEN pg_catalog.pg_size_pretty(pg_catalog.pg_database_size(d.datname))\n"
" ELSE 'No Access'\n"
" END as \"%s\"",
gettext_noop("Size"));
+ sizefunc = "pg_catalog.pg_database_size(d.datname)";
+ }
if (verbose && pset.sversion >= 80000)
appendPQExpBuffer(&buf,
",\n t.spcname as \"%s\"",
@@ -906,7 +917,10 @@ listAllDbs(const char *pattern, bool verbose)
processSQLNamePattern(pset.db, &buf, pattern, false, false,
NULL, "d.datname", NULL, NULL);
- appendPQExpBufferStr(&buf, "ORDER BY 1;");
+ if (pset.sort_by_size && sizefunc)
+ appendPQExpBuffer(&buf, "ORDER BY %s DESC, 1;", sizefunc);
+ else
+ appendPQExpBufferStr(&buf, "ORDER BY 1;");
res = PSQLexec(buf.data);
termPQExpBuffer(&buf);
if (!res)
@@ -3628,6 +3642,7 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
bool showMatViews = strchr(tabtypes, 'm') != NULL;
bool showSeq = strchr(tabtypes, 's') != NULL;
bool showForeign = strchr(tabtypes, 'E') != NULL;
+ const char *sizefunc = NULL;
PQExpBufferData buf;
PGresult *res;
@@ -3711,13 +3726,19 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
* 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"));
+ sizefunc = "pg_catalog.pg_table_size(c.oid)";
+ }
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"));
+ sizefunc = "pg_catalog.pg_relation_size(c.oid)";
+ }
appendPQExpBuffer(&buf,
",\n pg_catalog.obj_description(c.oid, 'pg_class') as \"%s\"",
@@ -3770,7 +3791,10 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
"n.nspname", "c.relname", NULL,
"pg_catalog.pg_table_is_visible(c.oid)");
- appendPQExpBufferStr(&buf, "ORDER BY 1,2;");
+ if (pset.sort_by_size && sizefunc)
+ appendPQExpBuffer(&buf, "ORDER BY %s DESC, 1, 2;", sizefunc);
+ else
+ appendPQExpBufferStr(&buf, "ORDER BY 1,2;");
res = PSQLexec(buf.data);
termPQExpBuffer(&buf);
@@ -3946,6 +3970,7 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
" JOIN d ON i.inhparent = d.oid)\n"
" SELECT pg_catalog.pg_size_pretty(sum(pg_catalog.pg_table_size("
"d.oid))) AS tps,\n"
+ " sum(pg_catalog.pg_table_size(d.oid)) AS rps,\n"
" pg_catalog.pg_size_pretty(sum("
"\n CASE WHEN d.level = 1"
" THEN pg_catalog.pg_table_size(d.oid) ELSE 0 END)) AS dps\n"
@@ -3961,6 +3986,7 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
" ELSE 0 END)) AS dps"
",\n pg_catalog.pg_size_pretty(sum("
"pg_catalog.pg_table_size(ppt.relid))) AS tps"
+ ",\n sum(pg_catalog.pg_table_size(ppt.relid)) AS rps"
"\n FROM pg_catalog.pg_partition_tree(c.oid) ppt) s");
}
}
@@ -3993,9 +4019,14 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
"n.nspname", "c.relname", NULL,
"pg_catalog.pg_table_is_visible(c.oid)");
- appendPQExpBuffer(&buf, "ORDER BY \"Schema\", %s%s\"Name\";",
- mixed_output ? "\"Type\" DESC, " : "",
- showNested || pattern ? "\"Parent name\" NULLS FIRST, " : "");
+ if (pset.sort_by_size && verbose)
+ appendPQExpBuffer(&buf, "ORDER BY %s%srps DESC",
+ mixed_output ? "\"Type\" DESC, " : "",
+ showNested || pattern ? "\"Parent name\" NULLS FIRST, " : "");
+ else
+ appendPQExpBuffer(&buf, "ORDER BY \"Schema\", %s%s\"Name\";",
+ mixed_output ? "\"Type\" DESC, " : "",
+ showNested || pattern ? "\"Parent name\" NULLS FIRST, " : "");
res = PSQLexec(buf.data);
termPQExpBuffer(&buf);
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index d9b982d3a0..fe17316624 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -342,7 +342,7 @@ helpVariables(unsigned short int pager)
* Windows builds currently print one more line than non-Windows builds.
* Using the larger number is fine.
*/
- output = PageOutput(158, pager ? &(pset.popt.topt) : NULL);
+ output = PageOutput(160, pager ? &(pset.popt.topt) : NULL);
fprintf(output, _("List of specially treated variables\n\n"));
@@ -411,6 +411,8 @@ helpVariables(unsigned short int pager)
" if set, end of line terminates SQL commands (same as -S option)\n"));
fprintf(output, _(" SINGLESTEP\n"
" single-step mode (same as -s option)\n"));
+ fprintf(output, _(" SORT_BY_SIZE\n"
+ " if set, verbose outputs are sorted by size\n"));
fprintf(output, _(" SQLSTATE\n"
" SQLSTATE of last query, or \"00000\" if no error\n"));
fprintf(output, _(" USER\n"
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 5be5091f0e..dc0033652b 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -127,6 +127,7 @@ typedef struct _psqlSettings
bool quiet;
bool singleline;
bool singlestep;
+ bool sort_by_size;
bool hide_tableam;
int fetch_count;
int histsize;
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 4730c73396..974c965875 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -884,6 +884,12 @@ singlestep_hook(const char *newval)
return ParseVariableBool(newval, "SINGLESTEP", &pset.singlestep);
}
+static bool
+sort_by_size_hook(const char *newval)
+{
+ return ParseVariableBool(newval, "SORT_BY_SIZE", &pset.sort_by_size);
+}
+
static char *
fetch_count_substitute_hook(char *newval)
{
@@ -1184,6 +1190,9 @@ EstablishVariableSpace(void)
SetVariableHooks(pset.vars, "SINGLESTEP",
bool_substitute_hook,
singlestep_hook);
+ SetVariableHooks(pset.vars, "SORT_BY_SIZE",
+ bool_substitute_hook,
+ sort_by_size_hook);
SetVariableHooks(pset.vars, "FETCH_COUNT",
fetch_count_substitute_hook,
fetch_count_hook);
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 3f7001fb69..427967be0a 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3737,7 +3737,7 @@ psql_completion(const char *text, int start, int end)
else if (TailMatchesCS("\\set", MatchAny))
{
if (TailMatchesCS("AUTOCOMMIT|ON_ERROR_STOP|QUIET|"
- "SINGLELINE|SINGLESTEP"))
+ "SINGLELINE|SINGLESTEP|SORT_BY_SIZE"))
COMPLETE_WITH_CS("on", "off");
else if (TailMatchesCS("COMP_KEYWORD_CASE"))
COMPLETE_WITH_CS("lower", "upper",
On Mon, 15 Jul 2019 at 06:12, Pavel Stehule <pavel.stehule@gmail.com> wrote:
Hi
pá 12. 7. 2019 v 15:10 odesílatel Fabien COELHO <coelho@cri.ensmp.fr> napsal:
Hello Pavel,
rebased patch attached
I prefer patches with a number rather than a date, if possible. For one
thing, there may be several updates in one day.About this version (20180708, probably v3): applies cleanly, compiles,
make check ok, doc build ok. No tests.attached version 4
It works for me on a few manual tests against a 11.4 server.
Documentation: if you say "\d*+", then it already applies to \db+ and
\dP+, so why listing them? Otherwise, state all commands or make it work
on all commands that have a size?About the text:
- remove , before "sorts"
- ... outputs by decreasing size, when size is displayed.
- add: When size is not displayed, the output is sorted by names.fixed
I still think that the object name should be kept as a secondary sort
criterion, in case of size equality, so that the output is deterministic.
Having plenty of objects of the same size out of alphabetical order looks
very strange.fixed
Regards
Pavel
I still do not like much the boolean approach. I understand that the name
approach has been rejected, and I can understand why.I've been thinking about another more generic interface, that I'm putting
here for discussion, I do not claim that it is a good idea. Probably could
fall under "over engineering", but it might not be much harder to
implement, and it solves a few potential problems.The idea is to add an option to \d commands, such as "\echo -n":
\dt+ [-o 1d,2a] ...
meaning do the \dt+, order by column 1 descending, column 2 ascending.
With this there would be no need for a special variable nor other
extensions to specify some ordering, whatever the user wishes.Maybe it could be "\dt+ [-o '1 DESC, 2 ASC'] ..." so that the string
is roughly used as an ORDER BY specification by the query, but it would be
longer to specify.It also solves the issue that if someone wants another sorting order we
would end with competing boolean variables such as SORT_BY_SIZE,
SORT_BY_TYPE, SORT_BY_SCHEMA, which would be pretty unpractical. The
boolean approach works for *one* sorting extension and breaks at the next
extension.Also, the boolean does not say that it is a descending order. I could be
interested in looking at the small tables.Another benefit for me is that I do not like much variables with side
effects, whereas with an explicit syntax there would be no such thing, the
user has what was asked for. Ok, psql is full of them, but I cannot say I
like it for that.The approach could be extended to specify a limit, eg \dt -l 10 would
add a LIMIT 10 on the query.Also, the implementation could be high enough so that the description
handlers would not have to deal with it individually, it could return
the query which would then be completed with SORT/LIMIT clauses before
being executed, possibly with a default order if none is specified.
I had a look at this patch, seems like a useful thing to have.
One clarification though,
What is the reason for compatibility with different versions in
listAllDbs and describeTablespaces, precisely
if (verbose && pset.sversion >= 90200)
+ {
appendPQExpBuffer(&buf,
",\n pg_catalog.pg_size_pretty(pg_catalog.pg_tablespace_size(oid))
AS \"%s\"",
gettext_noop("Size"));
+ sizefunc = "pg_catalog.pg_tablespace_size(oid)";
+ }
in describeTablespaces but
if (verbose && pset.sversion >= 80200)
+ {
appendPQExpBuffer(&buf,
",\n CASE WHEN pg_catalog.has_database_privilege(d.datname,
'CONNECT')\n"
" THEN
pg_catalog.pg_size_pretty(pg_catalog.pg_database_size(d.datname))\n"
" ELSE 'No Access'\n"
" END as \"%s\"",
gettext_noop("Size"));
+ sizefunc = "pg_catalog.pg_database_size(d.datname)";
+ }
in listAllDbs.--
Regards,
Rafia Sabih
On Fri, Jun 28, 2019 at 10:13 AM Pavel Stehule <pavel.stehule@gmail.com>
wrote:
Hi
I returned to possibility to sort output of \d* and \l by size. There was
more a experiments in this area, but without success. Last patch was
example of over engineering, and now, I try to implement this feature
simply how it is possible. I don't think so we need too complex solution -
if somebody needs specific report, then it is not hard to run psql with
"-E" option, get and modify used query (and use a power of SQL). But
displaying databases objects sorted by size is very common case.This proposal is based on new psql variable "SORT_BY_SIZE". This variable
will be off by default. The value of this variable is used only in verbose
mode (when the size is displayed - I don't see any benefit sort of size
without showing size). Usage is very simple and implementation too:\dt -- sorted by schema, name
\dt+ -- still sorted by schema, name\set SORT_BY_SIZE on
\dt -- sorted by schema, name (size is not calculated and is not visible)
\dt+ -- sorted by size\dt+ public.* -- sorted by size from schema public
Comments, notes?
Regards
Pavel
One oddity about pg_relation_size and pg_table_size is that they can be
easily blocked by user activity. In fact it happens to us often in
reporting environments and we have instead written different versions of
them that avoid the lock contention and still give "close enough" results.
This blocking could result in quite unexpected behavior, that someone uses
your proposed command and it never returns. Has that been considered as a
reality at least to be documented?
Thanks,
Jeremy
Hello Jeremy,
Comments, notes?
One oddity about pg_relation_size and pg_table_size is that they can be
easily blocked by user activity. In fact it happens to us often in
reporting environments and we have instead written different versions of
them that avoid the lock contention and still give "close enough" results.This blocking could result in quite unexpected behavior, that someone uses
your proposed command and it never returns. Has that been considered as a
reality at least to be documented?
ISTM that it does not change anything wrt the current behavior because of
the prudent lazy approach: the sorting is *only* performed when the size
is already available in one of the printed column.
Maybe the more general question could be "is there a caveat somewhere that
when doing \d.+ a user may have issues with locks because of the size
computations?".
--
Fabien.
On 2019-Jul-31, Rafia Sabih wrote:
I had a look at this patch, seems like a useful thing to have.
So the two initial questions for this patch are
1. Is this a feature we want?
2. Is the user interface correct?
I think the feature is useful, and Rafia also stated as much. Therefore
ISTM we're okay on that front.
As for the UI, Fabien thinks the patch adopts one that's far too
simplistic, and I agree. Fabien has proposed a number of different UIs,
but doesn't seem convinced of any of them. One of them was to have
"options" in the command,
\dt+ [-o 1d,2a]
Another idea is to use variables in a more general form. So instead of
Pavel's proposal of SORT_BY_SIZE=on we could do something like
SORT_BY=[list]
where the list after the equal sign consists of predetermined elements
(say SIZE, NAME, SCHEMA and so on) and indicates a specific column to
sort by. This is less succint than Fabien's idea, and in particular you
can't specify it in the command itself but have to set the variable
beforehand instead.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
čt 12. 9. 2019 v 0:01 odesílatel Alvaro Herrera from 2ndQuadrant <
alvherre@alvh.no-ip.org> napsal:
On 2019-Jul-31, Rafia Sabih wrote:
I had a look at this patch, seems like a useful thing to have.
So the two initial questions for this patch are
1. Is this a feature we want?
2. Is the user interface correct?I think the feature is useful, and Rafia also stated as much. Therefore
ISTM we're okay on that front.As for the UI, Fabien thinks the patch adopts one that's far too
simplistic, and I agree. Fabien has proposed a number of different UIs,
but doesn't seem convinced of any of them. One of them was to have
"options" in the command,
\dt+ [-o 1d,2a]
Another idea is to use variables in a more general form. So instead of
Pavel's proposal of SORT_BY_SIZE=on we could do something like
SORT_BY=[list]
where the list after the equal sign consists of predetermined elements
(say SIZE, NAME, SCHEMA and so on) and indicates a specific column to
sort by. This is less succint than Fabien's idea, and in particular you
can't specify it in the command itself but have to set the variable
beforehand instead.
for more generic design probably you need redesign psql report systems. You
cannot to use just ORDER BY 1,2 on some columns, but you need to produce
(and later hide) some content (for size).
So it can be unfunny complex patch. I finished sort inside pspg and I it
looks to be better solution, than increase complexity (and less
maintainability (due support old releases)).
Regards
Pavel
Show quoted text
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
pá 13. 9. 2019 v 9:35 odesílatel Pavel Stehule <pavel.stehule@gmail.com>
napsal:
čt 12. 9. 2019 v 0:01 odesílatel Alvaro Herrera from 2ndQuadrant <
alvherre@alvh.no-ip.org> napsal:On 2019-Jul-31, Rafia Sabih wrote:
I had a look at this patch, seems like a useful thing to have.
So the two initial questions for this patch are
1. Is this a feature we want?
2. Is the user interface correct?I think the feature is useful, and Rafia also stated as much. Therefore
ISTM we're okay on that front.As for the UI, Fabien thinks the patch adopts one that's far too
simplistic, and I agree. Fabien has proposed a number of different UIs,
but doesn't seem convinced of any of them. One of them was to have
"options" in the command,
\dt+ [-o 1d,2a]Another idea is to use variables in a more general form. So instead of
Pavel's proposal of SORT_BY_SIZE=on we could do something like
SORT_BY=[list]
where the list after the equal sign consists of predetermined elements
(say SIZE, NAME, SCHEMA and so on) and indicates a specific column to
sort by. This is less succint than Fabien's idea, and in particular you
can't specify it in the command itself but have to set the variable
beforehand instead.for more generic design probably you need redesign psql report systems.
You cannot to use just ORDER BY 1,2 on some columns, but you need to
produce (and later hide) some content (for size).So it can be unfunny complex patch. I finished sort inside pspg and I it
looks to be better solution, than increase complexity (and less
maintainability (due support old releases)).
I changed status for this patch to withdrawn
I like a idea with enhancing \dt about some clauses like " \dt+ [-o
1d,2a]". But it needs probably significant redesign of describe.c module.
Maybe implementation of some simple query generator for queries to system
catalogue can good.
Surely - this should be implemented from scratch - I am not a volunteer for
that.
Pavel
Show quoted text
Regards
Pavel
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services