|From:||Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>|
|To:||David Fetter <david(at)fetter(dot)org>|
|Cc:||Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>, Rafia Sabih <rafia(dot)pghackers(at)gmail(dot)com>, PostgreSQL Development <pgsql-hackers(at)postgresql(dot)org>|
|Subject:||Re: [PATCH v5] Show detailed table persistence in \dt+|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
David Fetter <david(at)fetter(dot)org> writes:
> [ v7-0001-Show-detailed-relation-persistence-in-dt.patch ]
I looked this over and had a few suggestions, as per attached v8:
* The persistence description values ought to be translatable, as
is the usual practice in describe.c. This is slightly painful
because it requires tweaking the translate_columns values in a
non-constant way, but it's not that bad.
* I dropped the "ELSE 'unknown'" bit in favor of just emitting NULL
if the persistence isn't recognized. This is the same way that the
table-type CASE just above does it, and I see no reason to be different.
Moreover, there are message-style-guidelines issues with what to print
if you do want to print something; "unknown" doesn't cut it.
* I also dropped the logic for pre-9.1 servers, because the existing
precedent in describeOneTableDetails() is that we only consider
relpersistence for >= 9.1, and I don't see a real good reason to
deviate from that. 9.0 and before are long out of support anyway.
If there aren't objections, I think v8 is committable.
regards, tom lane
|Next Message||Tom Lane||2019-07-02 19:13:30||Re: Add missing operator <->(box, point)|
|Previous Message||Alexander Korotkov||2019-07-02 18:55:40||Re: Add missing operator <->(box, point)|