psql's \d versus included-index-column feature

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: psql's \d versus included-index-column feature
Date: 2018-07-18 19:55:35
Message-ID: 21724.1531943735@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I noticed that psql's \d command doesn't do very well with included
index columns. Given the regression db's test case,

CREATE INDEX tbl_include_reg_idx ON tbl_include_reg (c1, c2) INCLUDE (c3, c4);

we get

regression=# \d tbl_include_reg_idx
Index "public.tbl_include_reg_idx"
Column | Type | Definition
--------+---------+------------
c1 | integer | c1
c2 | integer | c2
c3 | integer |
c4 | box |
btree, for table "public.tbl_include_reg"

So there are two problems with that: first, where's the definition
info for the included columns, and second, how's the user supposed
to tell which columns are key columns vs which are included?

(No, I don't accept the answer that you're supposed to know that
an omitted definition means an included column. For one thing,
that approach cannot scale to handle included expression columns.
For another, the documentation doesn't say any such thing.)

I looked into the reason why the definition is empty, and the answer
seems to be somebody's poorly-thought-through decision that
pg_get_indexdef_worker's attrsOnly flag could be made to serve two
purposes, with the result that pg_get_indexdef_ext with a nonzero column
argument will print nothing for an included column. That doesn't seem
like a sane (or documented) behavior either. So the attached patch
splits it into two flags, attrsOnly and keysOnly, and with that we get

regression=# \d tbl_include_reg_idx
Index "public.tbl_include_reg_idx"
Column | Type | Definition
--------+---------+------------
c1 | integer | c1
c2 | integer | c2
c3 | integer | c3
c4 | box | c4
btree, for table "public.tbl_include_reg"

which is better, but it's still not very clear what's what.
Do we want to consider that good enough, or do we want to
mark included columns in this display, and if so how?

One idea is to do it like this:

regression=# \d tbl_include_reg_idx
Index "public.tbl_include_reg_idx"
Column | Type | Definition
--------+---------+------------
c1 | integer | c1
c2 | integer | c2
c3 | integer | INCLUDE c3
c4 | box | INCLUDE c4
btree, for table "public.tbl_include_reg"

If we wanted to have pg_get_indexdef_ext() include "INCLUDE" in its output
for an included column, this could be achieved with just a couple more
lines of code. But that behavior seems rather ugly to me, and likely to
break other use-cases for pg_get_indexdef_ext(). So I think we likely
want to do this on the psql side instead, which will take more code but
also offers more flexibility for the display layout. For instance, maybe
we want something like

regression=# \d tbl_include_reg_idx
Index "public.tbl_include_reg_idx"
Column | Type | Key | Definition
--------+---------+------------------
c1 | integer | t | c1
c2 | integer | t | c2
c3 | integer | f | c3
c4 | box | f | c4
btree, for table "public.tbl_include_reg"

Thoughts?

regards, tom lane

Attachment Content-Type Size
fix-busted-pg_get_indexdef-behavior-1.patch text/x-diff 4.4 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2018-07-18 19:56:02 Re: [HACKERS] Re: [COMMITTERS] pgsql: Remove pgbench "progress" test pending solution of its timing is (fwd)
Previous Message Robert Haas 2018-07-18 19:45:31 Re: patch to allow disable of WAL recycling