Re: Showing index details with \d on psql - Mailing list pgsql-patches
From | Tom Lane |
---|---|
Subject | Re: Showing index details with \d on psql |
Date | |
Msg-id | 22541.1003015194@sss.pgh.pa.us Whole thread Raw |
In response to | Re: Showing index details with \d on psql ("Greg Sabino Mullane" <greg@turnstep.com>) |
Responses |
Re: Showing index details with \d on psql
|
List | pgsql-patches |
>> I don't like the '*' things. They look ugly and convey little >> real information. > They convey "this column is indexed" and also indicate in how many > indexes it appears. I tend to agree with Peter on that part ... the asterisks add more clutter than information. I also think that they could lead to ambiguity; for example, it's not obvious that the * is not part of the default clause where there's a default. I have a large number of problems with this part of your patch: char *s = _("Indexes:"); ! snprintf(buf, sizeof(buf), "%*s %s%s%s", ! (int)strlen(s), ! i == 0 ? s : "", ! PQgetvalue(result1, i, 0), ! strcmp(PQgetvalue(result1, i, 2), "t") == 0 ? _(" (primary key)") : "", ! strcmp(PQgetvalue(result1, i, 3), "t") == 0 ? _(" (unique)") : ""); ! ! char indexchar[5]; /* Should be plenty */ ! int indexnumber=0; ! char * indexlist = PQgetvalue(result1, i, 1); ! int j,found; ! for (j=0,found=0;j<=strlen(indexlist); j++) { ! if (indexlist[j] == 0 || indexlist[j] == 32) { ! indexnumber = atoi(indexchar); ! if (indexnumber>0) /* pg_class has a -2! */ ! { ! strcat(cells[(indexnumber-1) * cols + 2], ! cells[(indexnumber-1) * cols +2][0] ? " *" : "*"); ! strcat(buf, ++found==1 ? " (" : ", "); ! strcat(buf, cells[(indexnumber-1) * cols]); ! } ! indexchar[0] = '\0'; ! } ! else { strcat(indexchar,&indexlist[j]); } ! } ! if (found) /* must cover for pg_class again */ ! strcat(buf, ")"); footers[count_footers++] = xstrdup(buf); Gripe #1: declarations after the start of a block are a C++-ism. They are not legal in ANSI C. Gripe #2: what is indexchar[], why is it being used without initialization, and what is your justification for thinking 5 is enough space? Gripe #3: "32" is not a portable spelling of "' '". Gripe #4: looks to me like it will fail when indexes are on columns numbered 10 or above, because the loop will do strcat() multiple times. Gripe #5: doing the wrong thing on indexes that mention system columns (negative column numbers) isn't acceptable. You are really doing things quite the hard way here anyhow, since pg_get_indexdef would produce the info you want without so much work, and with less dependency in psql on backend catalog details. I'd suggest pulling the pg_get_indexdef result instead of indkey in the SELECT, and then just print the part after USING. BTW, "primary key" implies "unique", so I think it's not necessary to print both annotations for a primary key. regards, tom lane
pgsql-patches by date: