On Wed, Jun 15, 2011 at 1:03 AM, Noah Misch <noah@leadboat.com> wrote:
> [patch to avoid index rebuilds]
With respect to the documentation hunks, it seems to me that the first
hunk might be made clearer by leaving the paragraph of which it is a
part as-is, and adding another paragraph afterwards beginning with the
words "In addition". I am not sure whether the second hunk is
necessary at all. Doesn't the existing language cover the same
territory as what you've added?
I think that the variables in ATPostAlterTypeCleanup() could be better
named. They appear to be values, when in fact they are ListCells.
Honestly I'd probably just use l1 and l2, but if you want to insist on
some more mnemonic naming it should probably be something that sounds
vaguely list-ish.
As you no doubt expected, my eyes was immediately drawn to the
index-resurrection hack. Reviewing the thread, I see that you asked
about that in January and never got feedback. I have to say that what
you've done here looks like a pretty vile hack, but it's hard to say
for sure without knowing what to compare it against. You made
reference to this being smaller and simpler than updating the index
definition in place - can you give a sketch of what would need to be
done if we went that route instead?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company