On Wed, Apr 9, 2014 at 11:42 AM, Bruce Momjian <br...@momjian.us> wrote:
> On Wed, Apr  9, 2014 at 09:27:11AM -0400, Robert Haas wrote:
>> On Wed, Apr 9, 2014 at 1:02 AM, Tom Lane <t...@sss.pgh.pa.us> wrote:
>> > Robert Haas <robertmh...@gmail.com> writes:
>> >> Well, that's sorta my concern.  I mean, right now we've got people
>> >> saying "what the heck is a replica identity?".  But, if the logical
>> >> decoding stuff becomes popular, as I hope it will, that's going to be
>> >> an important thing for people to adjust, and the information needs to
>> >> be present in a clear and easily-understood way.  I haven't studied
>> >> the current code in detail so maybe it's fine.  I just want to make
>> >> sure we're not giving it second-class treatment solely on the basis
>> >> that it's new and people aren't using it yet.
>> >
>> > I think the proposal is "don't mention the property if it has the
>> > default value".  That's not second-class status, as long as people
>> > who know what the property is understand that behavior.  It's just
>> > conserving screen space.
>>
>> One thing that concerns me is that replica identity has a different
>> default for system tables (NOTHING) than for other tables (DEFAULT).
>> So when we say we're not going to display the default value, are we
>> going to display it when it's not NOTHING, when it's not DEFAULT, or
>> when it's not the actual default for that particular kind of table?
>
> We exclude pg_catalog from displaying Replica Identity due to this
> inconsistency.  I assume this was desired because you can't replicate
> system tables.  Is that true?  The current test is:
>
>         if ((tableinfo.relkind == 'r' || tableinfo.relkind == 'm') &&
>             /*
>              * No need to display default values;  we already display a
>              * REPLICA IDENTITY marker on indexes.
>              */
>             tableinfo.relreplident != 'd' && tableinfo.relreplident != 'i' &&
>             strcmp(schemaname, "pg_catalog") != 0)
>
> What might make more sense is this:
>
>         if ((tableinfo.relkind == 'r' || tableinfo.relkind == 'm') &&
>             /*
>              * No need to display default values;  we already display a
>              * REPLICA IDENTITY marker on indexes.
>              */
>             tableinfo.relreplident != 'i' &&
>             ((strcmp(schemaname, "pg_catalog") != 0 && tableinfo.relreplident 
> != 'd') ||
>              (strcmp(schemaname, "pg_catalog") == 0 && tableinfo.relreplident 
> != 'n')))

Well, this is why I think we should just display it always.  I don't
think users are going to remember the exact algorithm for whether or
not the line gets displayed, so you're just putting yourself in a
situation where the \d+ output doesn't actually inform the user.  If
you want to leave it out when it's "default" and show the "none" line
for catalog tables, that's OK by me too.  But calling one line of
output that displays important information "clutter" and only appears
when the user explicitly requests verbose mode (with \d+ rather than
\d) strikes me as as silly.

-- 
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

Reply via email to