PATCH: psql show index with type info

Started by Amos Birdalmost 9 years ago20 messages

amosbird@gmail.com

almost 9 years ago

1 attachment(s)

psql currently supports \di+ to view indexes,

The co
lumn "Type" is kinda useless (all equals to index). Representing
the actual index type will be more interesting,

I'm not sure where to add documentations about this patch or if needed one. Please help
me if you think this patch is useful.

Best regards,
Amos

Attachments:

0001_psql_show_index_with_type.v1.patchtext/x-diffDownload

diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index e2e4cbc..ac27662 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3169,7 +3169,7 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
                                          " WHEN 'r' THEN '%s'"
                                          " WHEN 'v' THEN '%s'"
                                          " WHEN 'm' THEN '%s'"
-                                         " WHEN 'i' THEN '%s'"
+                                         " WHEN 'i' THEN %s"
                                          " WHEN 'S' THEN '%s'"
                                          " WHEN 's' THEN '%s'"
                                          " WHEN 'f' THEN '%s'"
@@ -3181,7 +3181,7 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
                                          gettext_noop("table"),
                                          gettext_noop("view"),
                                          gettext_noop("materialized view"),
-                                         gettext_noop("index"),
+                                         gettext_noop("'index: '||(select amname from pg_am a where a.oid = c.relam)"),
                                          gettext_noop("sequence"),
                                          gettext_noop("special"),
                                          gettext_noop("foreign table"),

Stephen Frost

sfrost@snowman.net

almost 9 years ago

In reply to: Amos Bird (#1)

Re: PATCH: psql show index with type info

Greetings,

* Amos Bird (amosbird@gmail.com) wrote:

psql currently supports \di+ to view indexes,

List of relations
Schema | Name | Type | Owner | Table | Size | Description
--------+--------------------+-------+-------+---------+--------+-------------
public | ii | index | amos | i | 131 MB |
public | jj | index | amos | i | 12 MB |
public | kk | index | amos | i | 456 kB |
public | numbers_mod2 | index | amos | numbers | 10 MB |
public | numbers_mod2_btree | index | amos | numbers | 214 MB |
(5 rows)

The co
lumn "Type" is kinda useless (all equals to index). Representing
the actual index type will be more interesting,

Agreed.

Schema | Name | Type | Owner | Table | Size | Description
--------+--------------------+--------------+-------+---------+--------+-------------
public | ii | index: gist | amos | i | 131 MB |
public | jj | index: gin | amos | i | 12 MB |
public | kk | index: btree | amos | i | 456 kB |
public | numbers_mod2 | index: gin | amos | numbers | 10 MB |
public | numbers_mod2_btree | index: btree | amos | numbers | 214 MB |
(5 rows)

I'm not sure where to add documentations about this patch or if needed one. Please help
me if you think this patch is useful.

I'm not sure why it's useful to keep the 'index:'? I would suggest we
just drop that and keep only the actual index type (gist, gin, etc).

Thanks!

Stephen

Amos Bird

amosbird@gmail.com

almost 9 years ago

In reply to: Stephen Frost (#2)

Re: PATCH: psql show index with type info

Hello Stephen,

Well, the prefix is used to differentiate other \d commands, like
this,

regards,
Amos

Stephen Frost <sfrost@snowman.net> writes:

Greetings,

* Amos Bird (amosbird@gmail.com) wrote:

psql currently supports \di+ to view indexes,

List of relations
Schema | Name | Type | Owner | Table | Size | Description
--------+--------------------+-------+-------+---------+--------+-------------
public | ii | index | amos | i | 131 MB |
public | jj | index | amos | i | 12 MB |
public | kk | index | amos | i | 456 kB |
public | numbers_mod2 | index | amos | numbers | 10 MB |
public | numbers_mod2_btree | index | amos | numbers | 214 MB |
(5 rows)

The co
lumn "Type" is kinda useless (all equals to index). Representing
the actual index type will be more interesting,

Agreed.

Schema | Name | Type | Owner | Table | Size | Description
--------+--------------------+--------------+-------+---------+--------+-------------
public | ii | index: gist | amos | i | 131 MB |
public | jj | index: gin | amos | i | 12 MB |
public | kk | index: btree | amos | i | 456 kB |
public | numbers_mod2 | index: gin | amos | numbers | 10 MB |
public | numbers_mod2_btree | index: btree | amos | numbers | 214 MB |
(5 rows)

I'm not sure where to add documentations about this patch or if needed one. Please help
me if you think this patch is useful.

I'm not sure why it's useful to keep the 'index:'? I would suggest we
just drop that and keep only the actual index type (gist, gin, etc).

Thanks!

Stephen

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Stephen Frost

sfrost@snowman.net

almost 9 years ago

In reply to: Amos Bird (#3)

Re: PATCH: psql show index with type info

Amos,

* Amos Bird (amosbird@gmail.com) wrote:

Well, the prefix is used to differentiate other \d commands, like
this,

Ah, ok, fair enough.

Should we consider differentiating different table types also? I
suppose those are primairly just logged and unlogged, but I could see
that being useful information to know when doing a \dt. Not a big deal
either way though, and this patch stands on its own certainly.

Thanks!

Stephen

Amos Bird

amosbird@gmail.com

almost 9 years ago

In reply to: Stephen Frost (#4)

Re: PATCH: psql show index with type info

Yeah, I'm thinking about that too. Here is a full list of the original
type values,

"Schema"
"Name"
"table"
"view"
"materialized view"
"index"
"sequence"
"special"
"foreign table"
"table"

What else do you think will benefit from extra type information?

regards,
Amos

Stephen Frost <sfrost@snowman.net> writes:

Amos,

* Amos Bird (amosbird@gmail.com) wrote:

Well, the prefix is used to differentiate other \d commands, like
this,

Ah, ok, fair enough.

Should we consider differentiating different table types also? I
suppose those are primairly just logged and unlogged, but I could see
that being useful information to know when doing a \dt. Not a big deal
either way though, and this patch stands on its own certainly.

Thanks!

Stephen

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Ashutosh Bapat

ashutosh.bapat@enterprisedb.com

almost 9 years ago

In reply to: Amos Bird (#3)

Re: PATCH: psql show index with type info

On Mon, Mar 6, 2017 at 7:54 PM, Amos Bird <amosbird@gmail.com> wrote:

Hello Stephen,

Well, the prefix is used to differentiate other \d commands, like
this,

amos=# \ditv
List of relations
Schema | Name | Type | Owner | Table
--------+--------------------+--------------+-------+---------
public | i | table | amos |
public | ii | index: gist | amos | i
public | j | table | amos |
public | jj | index: gin | amos | i
public | jp | index: btree | amos | i
public | js | index: brin | amos | i
public | numbers | table | amos |
public | numbers_mod2 | index: gin | amos | numbers
public | numbers_mod2_btree | index: btree | amos | numbers
public | ts | table | amos |
(10 rows)

The header for this table is "list of relations", so type gets
associated with relations indicated type of relation. btree: gin as a
type of relation doesn't sound really great. Instead we might want to
add another column "access method" and specify the access method used
for that relation. But then only indexes seem to have access methods
per pg_class.h.

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Stephen Frost

sfrost@snowman.net

almost 9 years ago

In reply to: Ashutosh Bapat (#6)

Re: PATCH: psql show index with type info

Ashutosh,

* Ashutosh Bapat (ashutosh.bapat@enterprisedb.com) wrote:

On Mon, Mar 6, 2017 at 7:54 PM, Amos Bird <amosbird@gmail.com> wrote:

Well, the prefix is used to differentiate other \d commands, like
this,

amos=# \ditv
List of relations
Schema | Name | Type | Owner | Table
--------+--------------------+--------------+-------+---------
public | i | table | amos |
public | ii | index: gist | amos | i
public | j | table | amos |
public | jj | index: gin | amos | i
public | jp | index: btree | amos | i
public | js | index: brin | amos | i
public | numbers | table | amos |
public | numbers_mod2 | index: gin | amos | numbers
public | numbers_mod2_btree | index: btree | amos | numbers
public | ts | table | amos |
(10 rows)

The header for this table is "list of relations", so type gets
associated with relations indicated type of relation. btree: gin as a
type of relation doesn't sound really great.

The type is 'index', we're just adding a sub-type here to clarify the
kind of index it is.

Instead we might want to
add another column "access method" and specify the access method used
for that relation. But then only indexes seem to have access methods
per pg_class.h.

Right, I don't think having an extra column which is going to be NULL a
large amount of the time is good. The approach taken by Amos seems like
a good one to me, to have the type still be 'index' but with a sub-type
indicating the access method.

Thanks!

Stephen

Ashutosh Bapat

ashutosh.bapat@enterprisedb.com

almost 9 years ago

In reply to: Stephen Frost (#7)

Re: PATCH: psql show index with type info

On Wed, Mar 8, 2017 at 7:00 PM, Stephen Frost <sfrost@snowman.net> wrote:

Ashutosh,

* Ashutosh Bapat (ashutosh.bapat@enterprisedb.com) wrote:

On Mon, Mar 6, 2017 at 7:54 PM, Amos Bird <amosbird@gmail.com> wrote:

Well, the prefix is used to differentiate other \d commands, like
this,

amos=# \ditv
List of relations
Schema | Name | Type | Owner | Table
--------+--------------------+--------------+-------+---------
public | i | table | amos |
public | ii | index: gist | amos | i
public | j | table | amos |
public | jj | index: gin | amos | i
public | jp | index: btree | amos | i
public | js | index: brin | amos | i
public | numbers | table | amos |
public | numbers_mod2 | index: gin | amos | numbers
public | numbers_mod2_btree | index: btree | amos | numbers
public | ts | table | amos |
(10 rows)

The header for this table is "list of relations", so type gets
associated with relations indicated type of relation. btree: gin as a
type of relation doesn't sound really great.

The type is 'index', we're just adding a sub-type here to clarify the
kind of index it is.

Instead we might want to
add another column "access method" and specify the access method used
for that relation. But then only indexes seem to have access methods
per pg_class.h.

Right, I don't think having an extra column which is going to be NULL a
large amount of the time is good. The approach taken by Amos seems like
a good one to me, to have the type still be 'index' but with a sub-type
indicating the access method.

Ok. Seems like a good trade-off.

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Peter Eisentraut

peter.eisentraut@2ndquadrant.com

almost 9 years ago

In reply to: Ashutosh Bapat (#6)

Re: PATCH: psql show index with type info

On 3/8/17 04:11, Ashutosh Bapat wrote:

The header for this table is "list of relations", so type gets
associated with relations indicated type of relation. btree: gin as a
type of relation doesn't sound really great. Instead we might want to
add another column "access method" and specify the access method used
for that relation.

I like that.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#10

Peter Eisentraut

peter.eisentraut@2ndquadrant.com

almost 9 years ago

In reply to: Stephen Frost (#7)

Re: PATCH: psql show index with type info

On 3/8/17 08:30, Stephen Frost wrote:

Right, I don't think having an extra column which is going to be NULL a
large amount of the time is good.

Note that \di already has a column "Table" that is null for something
that is not an index. So I don't think this argument is very strong.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#11

Stephen Frost

sfrost@snowman.net

almost 9 years ago

In reply to: Peter Eisentraut (#9)

Re: PATCH: psql show index with type info

Peter,

* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:

On 3/8/17 04:11, Ashutosh Bapat wrote:

The header for this table is "list of relations", so type gets
associated with relations indicated type of relation. btree: gin as a
type of relation doesn't sound really great. Instead we might want to
add another column "access method" and specify the access method used
for that relation.

I like that.

When it will be NULL for all of the regular tables..?

I'd rather have the one Type column that just included the access method
when there is one to include.

Thanks!

Stephen

#12

Stephen Frost

sfrost@snowman.net

almost 9 years ago

In reply to: Peter Eisentraut (#10)

Re: PATCH: psql show index with type info

* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:

On 3/8/17 08:30, Stephen Frost wrote:

Right, I don't think having an extra column which is going to be NULL a
large amount of the time is good.

Note that \di already has a column "Table" that is null for something
that is not an index. So I don't think this argument is very strong.

That's an interesting point.

I think what I find most odd about all of this is that \dti and \dit
work at all, and give a different set of columns from \dt. We don't
document that combining those works in \?, as far as I can see, and
other combinations don't work, just this.

In any case, I won't push very hard on this, it's useful information to
include and we should do so.

Thanks!

Stephen

#13

Robert Haas

robertmhaas@gmail.com

almost 9 years ago

In reply to: Stephen Frost (#4)

Re: PATCH: psql show index with type info

On Mon, Mar 6, 2017 at 9:30 AM, Stephen Frost <sfrost@snowman.net> wrote:

* Amos Bird (amosbird@gmail.com) wrote:

Well, the prefix is used to differentiate other \d commands, like
this,

Ah, ok, fair enough.

Should we consider differentiating different table types also? I
suppose those are primairly just logged and unlogged, but I could see
that being useful information to know when doing a \dt. Not a big deal
either way though, and this patch stands on its own certainly.

Logged vs. unlogged isn't the same thing as identifying the access
method. Anything with storage can come in logged, unlogged, and
temporary varieties, but an access method is essentially a way of
saying that one object has a different physical layout than another.
Right now there is, indeed, only one heap access method, but some of
my colleagues and I are scheming over how to change that.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#14

Stephen Frost

sfrost@snowman.net

almost 9 years ago

In reply to: Robert Haas (#13)

Re: PATCH: psql show index with type info

* Robert Haas (robertmhaas@gmail.com) wrote:

On Mon, Mar 6, 2017 at 9:30 AM, Stephen Frost <sfrost@snowman.net> wrote:

* Amos Bird (amosbird@gmail.com) wrote:

Well, the prefix is used to differentiate other \d commands, like
this,

Ah, ok, fair enough.

Should we consider differentiating different table types also? I
suppose those are primairly just logged and unlogged, but I could see
that being useful information to know when doing a \dt. Not a big deal
either way though, and this patch stands on its own certainly.

Logged vs. unlogged isn't the same thing as identifying the access
method. Anything with storage can come in logged, unlogged, and
temporary varieties, but an access method is essentially a way of
saying that one object has a different physical layout than another.
Right now there is, indeed, only one heap access method, but some of
my colleagues and I are scheming over how to change that.

Fair enough, sounds like a good reason to add an Access Method column to
me.

Thanks!

Stephen

#15

Fabien COELHO

coelho@cri.ensmp.fr

over 8 years ago

In reply to: Amos Bird (#1)

Re: PATCH: psql show index with type info

I'm not sure where to add documentations about this patch or if needed one. Please help
me if you think this patch is useful.

This patch does not apply anymore. Are you planning to update it?

--
Fabien.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#16

Amos Bird

amosbird@gmail.com

over 8 years ago

In reply to: Fabien COELHO (#15)

1 attachment(s)

Re: PATCH: psql show index with type info

Done. Would you like to be the reviewer? Thanks!

Attachments:

0001_psql_show_index_with_type.v1.patchtext/x-diffDownload

diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 0f9f497..a6dc599 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -3284,7 +3284,7 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
 					  " WHEN " CppAsString2(RELKIND_RELATION) " THEN '%s'"
 					  " WHEN " CppAsString2(RELKIND_VIEW) " THEN '%s'"
 					  " WHEN " CppAsString2(RELKIND_MATVIEW) " THEN '%s'"
-					  " WHEN " CppAsString2(RELKIND_INDEX) " THEN '%s'"
+					  " WHEN " CppAsString2(RELKIND_INDEX) " THEN %s"
 					  " WHEN " CppAsString2(RELKIND_SEQUENCE) " THEN '%s'"
 					  " WHEN 's' THEN '%s'"
 					  " WHEN " CppAsString2(RELKIND_FOREIGN_TABLE) " THEN '%s'"
@@ -3296,7 +3296,7 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
 					  gettext_noop("table"),
 					  gettext_noop("view"),
 					  gettext_noop("materialized view"),
-					  gettext_noop("index"),
+					  gettext_noop("'index: '||(select amname from pg_am a where a.oid = c.relam)"),
 					  gettext_noop("sequence"),
 					  gettext_noop("special"),
 					  gettext_noop("foreign table"),

#17

Fabien COELHO

coelho@cri.ensmp.fr

over 8 years ago

In reply to: Amos Bird (#16)

Re: PATCH: psql show index with type info

Done.

Ok. The file should be named "v2".

Would you like to be the reviewer?

Dunno. At least I wanted to have a look at it!

My 0.02ï¿œ:

I think that the improvement provided is worthwhile.

Two questions: Why no documentation update? Why no non-regressions
tests?

As far as the output is concerned, ISTM that "btree index", "hash index"
and so would sound nicer than "index: sub-type".

I'm wondering about the sub-query implementation: Maybe an outer join
could bring the same result with a more relational & elegant query.

The "gettext_noop" call does not seem to make sense: the point is to
translate the string, and there is no way to translate "index: <some sub
query>". The result of the query should be translated if this is what is
wanted, and that can only be by hand:

CASE amname || ' index'
WHEN 'hash index' THEN '%s' ...
WHEN ...
ELSE amname || ' index' # untranslated...
END

gettext_noop('hash index') # get translation for 'hash index'
...

--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#18

Amos Bird

amosbird@gmail.com

over 8 years ago

In reply to: Fabien COELHO (#17)

Re: PATCH: psql show index with type info

Ah, thanks for the suggestions. I'll revise this patch soon :)

Fabien COELHO <coelho@cri.ensmp.fr> writes:

Done.

Ok. The file should be named "v2".

Would you like to be the reviewer?

Dunno. At least I wanted to have a look at it!

My 0.02€:

I think that the improvement provided is worthwhile.

Two questions: Why no documentation update? Why no non-regressions
tests?

As far as the output is concerned, ISTM that "btree index", "hash index"
and so would sound nicer than "index: sub-type".

I'm wondering about the sub-query implementation: Maybe an outer join
could bring the same result with a more relational & elegant query.

The "gettext_noop" call does not seem to make sense: the point is to
translate the string, and there is no way to translate "index: <some sub
query>". The result of the query should be translated if this is what is
wanted, and that can only be by hand:

CASE amname || ' index'
WHEN 'hash index' THEN '%s' ...
WHEN ...
ELSE amname || ' index' # untranslated...
END

gettext_noop('hash index') # get translation for 'hash index'
...

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#19

Daniel Gustafsson

daniel@yesql.se

over 8 years ago

In reply to: Amos Bird (#18)

Re: PATCH: psql show index with type info

On 18 Apr 2017, at 05:13, Amos Bird <amosbird@gmail.com> wrote:

Ah, thanks for the suggestions. I'll revise this patch soon :)

Have you had a chance to revise the patch to address the review comments such
that the patch can move forward during this Commitfest?

cheers ./daniel

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#20

Daniel Gustafsson

daniel@yesql.se

over 8 years ago

In reply to: Daniel Gustafsson (#19)

Re: PATCH: psql show index with type info

On 13 Sep 2017, at 01:24, Daniel Gustafsson <daniel@yesql.se> wrote:

On 18 Apr 2017, at 05:13, Amos Bird <amosbird@gmail.com> wrote:

Ah, thanks for the suggestions. I'll revise this patch soon :)

Have you had a chance to revise the patch to address the review comments such
that the patch can move forward during this Commitfest?

Since the patch was Waiting for author without being updated during the
commitfest, it has been Returned with Feedback.

cheers ./daniel

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers