Re: [HACKERS] PATCH: psql show index with type info
> On 13 Sep 2017, at 01:24, Daniel Gustafssonwrote: > >> On 18 Apr 2017, at 05:13, Amos Bird 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
Re: [HACKERS] PATCH: psql show index with type info
> On 18 Apr 2017, at 05:13, Amos Birdwrote: > > 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
Re: [HACKERS] PATCH: psql show index with type info
Ah, thanks for the suggestions. I'll revise this patch soon :) Fabien COELHOwrites: >> 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: 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
Re: [HACKERS] 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: 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
Re: [HACKERS] PATCH: psql show index with type info
Done. Would you like to be the reviewer? Thanks! 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"), . Fabien COELHOwrites: >> 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? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] 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
Re: [HACKERS] PATCH: psql show index with type info
* Robert Haas (robertmh...@gmail.com) wrote: > On Mon, Mar 6, 2017 at 9:30 AM, Stephen Frostwrote: > > * Amos Bird (amosb...@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 signature.asc Description: Digital signature
Re: [HACKERS] PATCH: psql show index with type info
On Mon, Mar 6, 2017 at 9:30 AM, Stephen Frostwrote: > * Amos Bird (amosb...@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
Re: [HACKERS] PATCH: psql show index with type info
* Peter Eisentraut (peter.eisentr...@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 signature.asc Description: Digital signature
Re: [HACKERS] PATCH: psql show index with type info
Peter, * Peter Eisentraut (peter.eisentr...@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 signature.asc Description: Digital signature
Re: [HACKERS] 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
Re: [HACKERS] 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
Re: [HACKERS] PATCH: psql show index with type info
On Wed, Mar 8, 2017 at 7:00 PM, Stephen Frostwrote: > Ashutosh, > > * Ashutosh Bapat (ashutosh.ba...@enterprisedb.com) wrote: >> On Mon, Mar 6, 2017 at 7:54 PM, Amos Bird 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
Re: [HACKERS] PATCH: psql show index with type info
Ashutosh, * Ashutosh Bapat (ashutosh.ba...@enterprisedb.com) wrote: > On Mon, Mar 6, 2017 at 7:54 PM, Amos Birdwrote: > > 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 signature.asc Description: Digital signature
Re: [HACKERS] PATCH: psql show index with type info
On Mon, Mar 6, 2017 at 7:54 PM, Amos Birdwrote: > > 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
Re: [HACKERS] 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 Frostwrites: > Amos, > > * Amos Bird (amosb...@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
Re: [HACKERS] PATCH: psql show index with type info
Amos, * Amos Bird (amosb...@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 signature.asc Description: Digital signature
Re: [HACKERS] PATCH: psql show index with type info
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) regards, Amos Stephen Frostwrites: > Greetings, > > * Amos Bird (amosb...@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
Re: [HACKERS] PATCH: psql show index with type info
Greetings, * Amos Bird (amosb...@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 signature.asc Description: Digital signature
[HACKERS] PATCH: psql show index with type info
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, 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. Best regards, Amos 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"), -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers