Prev: On Scalability
Next: knngist - 0.8
From: "Kevin Grittner" on 29 Jul 2010 19:56
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
 | 
Pages: 1
Prev: On Scalability
Next: knngist - 0.8