Thread
Commits
-
Fix pg_get_indexdef()'s behavior for included index columns.
- 0f49a2de96e7 11.0 landed
- 028e3da29494 12.0 landed
-
Improve psql's \d command to show whether index columns are key columns.
- f4a5ce359587 11.0 landed
- 90371a24a536 12.0 landed
-
psql's \d versus included-index-column feature
Tom Lane <tgl@sss.pgh.pa.us> — 2018-07-18T19:55:35Z
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
-
Re: psql's \d versus included-index-column feature
David G. Johnston <david.g.johnston@gmail.com> — 2018-07-18T20:14:30Z
On Wed, Jul 18, 2018 at 12:55 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > 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" > +1 for the additional column indicating whether the column is being treated as key data or supplemental included data. -1 for printing a boolean t/f; would rather spell it out: CASE WHEN "Key" THEN 'Key' ELSE 'Included' END AS "Data" We're not hurting for horizontal space here and in any case I'd rather save others the eye strain of having to distinguish between lowercase "f" and "t". David J.
-
Re: psql's \d versus included-index-column feature
Alvaro Herrera <alvherre@2ndquadrant.com> — 2018-07-18T20:26:32Z
On 2018-Jul-18, David G. Johnston wrote: > -1 for printing a boolean t/f; would rather spell it out: > > CASE WHEN "Key" THEN 'Key' ELSE 'Included' END AS "Data" +1 -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
-
Re: psql's \d versus included-index-column feature
Tom Lane <tgl@sss.pgh.pa.us> — 2018-07-18T20:46:16Z
Alvaro Herrera <alvherre@2ndquadrant.com> writes: > On 2018-Jul-18, David G. Johnston wrote: >> -1 for printing a boolean t/f; would rather spell it out: >> >> CASE WHEN "Key" THEN 'Key' ELSE 'Included' END AS "Data" > +1 I can sympathize with the eyestrain argument against t/f, but the above doesn't seem like an improvement --- in particular, "Data" as the column header seems quite content-free. My counterproposal is to keep "Key" as the header and use "Yes"/"No" as the values. I'd be OK with "Key"/"Included" as the values if someone can propose an on-point column header to go with those. regards, tom lane
-
Re: psql's \d versus included-index-column feature
Alvaro Herrera <alvherre@2ndquadrant.com> — 2018-07-18T21:05:44Z
On 2018-Jul-18, Tom Lane wrote: > I can sympathize with the eyestrain argument against t/f, but the > above doesn't seem like an improvement --- in particular, "Data" > as the column header seems quite content-free. My counterproposal > is to keep "Key" as the header and use "Yes"/"No" as the values. I think "Key: no" is a bit obscure -- using "included" is a bit more self-documenting and lends better to documentation searches. > I'd be OK with "Key"/"Included" as the values if someone can > propose an on-point column header to go with those. "Role"? -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
-
Re: psql's \d versus included-index-column feature
Alexander Korotkov <a.korotkov@postgrespro.ru> — 2018-07-18T23:11:11Z
On Wed, Jul 18, 2018 at 11:14 PM David G. Johnston < david.g.johnston@gmail.com> wrote: > On Wed, Jul 18, 2018 at 12:55 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > >> >> 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" >> > > +1 for the additional column indicating whether the column is being > treated as key data or supplemental included data. > +1 And especially I don't think we should place word "INCLUDE" to the definition column. -1 for printing a boolean t/f; would rather spell it out: > IMHO, t/f have advantage of brevity. From my point of view, covering indexes are not so evident feature. So, users need to spend some time reading documentation before realizing what they are and how to use them. So, I don't expect that short designation of INCLUDE columns as "non-key" (Key == false) columns could be discouraging here. ------ Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
-
Re: psql's \d versus included-index-column feature
Oleksandr Shulgin <oleksandr.shulgin@zalando.de> — 2018-07-19T08:04:01Z
On Thu, Jul 19, 2018 at 1:11 AM Alexander Korotkov < a.korotkov@postgrespro.ru> wrote: > On Wed, Jul 18, 2018 at 11:14 PM David G. Johnston < > david.g.johnston@gmail.com> wrote: > >> On Wed, Jul 18, 2018 at 12:55 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> >>> >>> 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" >>> >> >> +1 for the additional column indicating whether the column is being >> treated as key data or supplemental included data. >> > > +1 > And especially I don't think we should place word "INCLUDE" to the > definition column. > > -1 for printing a boolean t/f; would rather spell it out: >> > > IMHO, t/f have advantage of brevity. From my point of view, covering > indexes are not so evident feature. So, users need to spend some time > reading documentation before realizing what they are and how to use them. > So, I don't expect that short designation of INCLUDE columns as "non-key" > (Key == false) columns could be discouraging here. > I don't think there is an established practice on how to display this sort of info, but I see that both styles already exist, namely: =# \dL List of languages Name | Owner | Trusted | Description ------------+----------+---------+------------------------------ plpgsql | postgres | t | PL/pgSQL procedural language plproxy | postgres | f | ... and =# \dC List of casts Source type | Target type | Function | Implicit? -----------------------------+-----------------------------+--------------------+--------------- abstime | date | date | in assignment abstime | integer | (binary coercible) | no abstime | timestamp without time zone | timestamp | yes ... I like the second option more, for readability reasons and because it is easier to extend if ever needed. Given that the documentation refers to included columns as "non-key columns", it seems natural to me to name the \d output column "Key?" and use "yes/no" as the values. Regards, -- Alex -
Re: psql's \d versus included-index-column feature
Tom Lane <tgl@sss.pgh.pa.us> — 2018-07-19T15:11:00Z
Oleksandr Shulgin <oleksandr.shulgin@zalando.de> writes: > I don't think there is an established practice on how to display this sort > of info, but I see that both styles already exist, namely: > =# \dL > List of languages > Name | Owner | Trusted | Description > ------------+----------+---------+------------------------------ > plpgsql | postgres | t | PL/pgSQL procedural language > plproxy | postgres | f | > ... > and > =# \dC > List of casts > Source type | Target type | Function > | Implicit? > -----------------------------+-----------------------------+--------------------+--------------- > abstime | date | date > | in assignment > abstime | integer | (binary > coercible) | no > abstime | timestamp without time zone | timestamp > | yes > ... > I like the second option more, for readability reasons and because it is > easier to extend if ever needed. > Given that the documentation refers to included columns as "non-key > columns", it seems natural to me to name the \d output column "Key?" and > use "yes/no" as the values. WFM, anyone want to argue against? regards, tom lane
-
Re: psql's \d versus included-index-column feature
David G. Johnston <david.g.johnston@gmail.com> — 2018-07-19T15:44:10Z
On Thursday, July 19, 2018, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > > Given that the documentation refers to included columns as "non-key > > columns", it seems natural to me to name the \d output column "Key?" and > > use "yes/no" as the values. > > WFM, anyone want to argue against? > Works for me as well. David J.