Thread

1. review patch: Distinguish between unique indexes and unique constraints

   Kevin Grittner <kevin.grittner@wicourts.gov> — 2010-07-29T23:56:00Z

   The patch is in context diff format and applies cleanly.  No doc
   changes were included.  Arguably there should be a mention in the
   documentation for psql's \d+ commend, but since the number of child
   tables and the display of reloptions aren't mentioned, perhaps we're
   not trying to list *all* the differences the + makes.  If those
   don't merit mention, this doesn't.
    
   The patch implements what it's supposed to, there was consensus on
   the list that we want it, we don't already have it, the SQL spec
   doesn't apply to psql's backslash commands, and it was added just
   for the verbose listing as requested (with no objection).  There
   are no pg_dump issues.  The only danger is that someone is depending
   on the current \d+ format and will be surprised at the new
   distinction between unique indexes and unique constraints.  All
   bases seem to me to be covered.
    
   The feature works as advertised, I saw no problem corner cases, no
   assertion failures or crashes.
    
   The patch causes no noticeable performance change, doesn't claim to
   affect performance, and will not slow down anything else.
    
   The patch does not follow coding guidelines, as it places the
   opening brace for an "if" block on the same line as the "if".
   There are no portability issues; it should work everywhere that
   current backslash commands work.  The purpose of the code is obvious
   enough to not require any comment lines.  It does what it says,
   correctly.  It doesn't produce compiler warnings and does not crash.
    
   It fits together coherently with all else, and introduces no new
   interdependencies.
    
   I am attaching a new version of the patch which fixes the formatting
   issue and rearranges the code slightly in a way which I find more
   readable.  (I leave it to the committer to determine which
   arrangement better suits.)
    
   I am marking this "Ready for Committer".
    
   -Kevin
   

2. Re: review patch: Distinguish between unique indexes and unique constraints

   Robert Haas <robertmhaas@gmail.com> — 2010-08-01T01:11:53Z

   On Thu, Jul 29, 2010 at 7:56 PM, Kevin Grittner
   <Kevin.Grittner@wicourts.gov> wrote:
   > The patch is in context diff format and applies cleanly.  No doc
   > changes were included.  Arguably there should be a mention in the
   > documentation for psql's \d+ commend, but since the number of child
   > tables and the display of reloptions aren't mentioned, perhaps we're
   > not trying to list *all* the differences the + makes.  If those
   > don't merit mention, this doesn't.
   >
   > The patch implements what it's supposed to, there was consensus on
   > the list that we want it, we don't already have it, the SQL spec
   > doesn't apply to psql's backslash commands, and it was added just
   > for the verbose listing as requested (with no objection).  There
   > are no pg_dump issues.  The only danger is that someone is depending
   > on the current \d+ format and will be surprised at the new
   > distinction between unique indexes and unique constraints.  All
   > bases seem to me to be covered.
   >
   > The feature works as advertised, I saw no problem corner cases, no
   > assertion failures or crashes.
   >
   > The patch causes no noticeable performance change, doesn't claim to
   > affect performance, and will not slow down anything else.
   >
   > The patch does not follow coding guidelines, as it places the
   > opening brace for an "if" block on the same line as the "if".
   > There are no portability issues; it should work everywhere that
   > current backslash commands work.  The purpose of the code is obvious
   > enough to not require any comment lines.  It does what it says,
   > correctly.  It doesn't produce compiler warnings and does not crash.
   >
   > It fits together coherently with all else, and introduces no new
   > interdependencies.
   >
   > I am attaching a new version of the patch which fixes the formatting
   > issue and rearranges the code slightly in a way which I find more
   > readable.  (I leave it to the committer to determine which
   > arrangement better suits.)
   >
   > I am marking this "Ready for Committer".
   
   I have committed this patch with a few changes.  First, I felt that
   there was little point in showing this detail only in verbose mode;
   indeed, it seems like that could be confusing in some circumstances.
   (I thought I checked this was an index not a constraint?  Oh, crap, I
   forgot to say \d+).  So I removed that check.  Second, I felt that
   UNIQUE, CONSTRAINT, btree (a) was one too many commas, so I made it
   just say UNIQUE CONSTRAINT, btree (a) instead.  I didn't see any
   discussion of either of these points in the previous discussion of
   this patch, so hopefully neither change is objectionable (but if it
   is, we can revisit it, if someone thinks I'm all wet!).
   
   Thanks to Josh for the patch and Kevin for the review; I think this is
   a good change.
   
   -- 
   Robert Haas
   EnterpriseDB: http://www.enterprisedb.com
   The Enterprise Postgres Company