Re: proposal - patch: psql - sort_by_size - Mailing list pgsql-hackers

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