From: | Teodor Sigaev <teodor(at)sigaev(dot)ru> |
---|---|
To: | Daniel Verite <daniel(at)manitou-mail(dot)org>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [patch] Proposal for \crosstabview in psql |
Date: | 2016-02-08 15:25:27 |
Message-ID: | 56B8B367.7050401@sigaev.ru |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi!
Interesting feature, but it's not very obvious how to use it. I'd like to see
some example(s) in documentation.
And I see an implementation of AVL tree in psql source code
(src/bin/psql/crosstabview.c). Postgres already has a Red-Black tree
implementation in
src/include/lib/rbtree.h and src/backend/lib/rbtree.c. Is any advantage of using
AVL tree here? I have some doubt about that and this implementation, obviously,
will not be well tested. But I see in comments that implementation is reduced to
insert only and it doesn't use the fact of ordering tree, so, even hash could be
used.
Daniel Verite wrote:
> Pavel Stehule wrote:
>
>> 1. maybe we can decrease name to shorter "crossview" ?? I am happy with
>> crosstabview too, just crossview is correct too, and shorter
>
> I'm in favor of keeping crosstabview. It's more explicit, only
> 3 characters longer and we have tab completion anyway.
>
>> 2. Columns used for ordering should not be displayed by default. I can live
>> with current behave, but hiding ordering columns is much more practical for
>> me
>
> I can see why, but I'm concerned about a consequence:
> say we have 4 columns A,B,C,D and user does \crosstabview +A:B +C:D
> If B and D are excluded by default, then there's nothing
> left to display inside the grid.
> It doesn't feel quite right. There's something counter-intuitive
> in the fact that values in the grid would disappear depending on
> whether and how headers are sorted.
> With the 3rd argument, we let the user decide what they want
> to see.
>
>> 3. This code is longer, so some regress tests are recommended - attached
>> simple test case
>
> I've added a few regression tests to the psql testsuite
> based on your sample data. New patch with these tests
> included is attached, make check passes.
>
> Best regards,
>
>
>
>
--
Teodor Sigaev E-mail: teodor(at)sigaev(dot)ru
WWW: http://www.sigaev.ru/
From | Date | Subject | |
---|---|---|---|
Next Message | Fujii Masao | 2016-02-08 15:48:57 | Re: Support for N synchronous standby servers - take 2 |
Previous Message | Andres Freund | 2016-02-08 15:17:57 | Re: a raft of parallelism-related bug fixes |