On Thu, Nov 14, 2013 at 8:46 AM, Andres Freund <andres@2ndquadrant.com> wrote:
> [ new patches ]
Here's an updated version of patch #2. I didn't really like the
approach you took in the documentation, so I revised it.
Apart from that, I spent a lot of time looking at
HeapSatisfiesHOTandKeyUpdate. I'm not very happy with your changes.
The idea seems to be that we'll iterate through all of the HOT columns
regardless, but that might be very inefficient. Suppose there are 100
HOT columns, the last one is the only key column, and only the first
one has been modified. Once we look at #1 and determine that it's not
HOT, we should zoom forward and skip over the next 98, and only look
at the last one; your version does not behave like that.
I think there's also some confusion in your version about what ends up
in the attnum values: they're normally adjusted by
FirstLowInvalidHeapAttributeNumber, but when bms_first_member returns
-1 then they're not. But that's not a great thing, because -1 is
actually a valid attribute number. I've taken a crack at rewriting
this logic, and the result looks cleaner and simpler to me, but I
haven't tested it beyond the fact that it passes make check. See what
you think.
I haven't completely reviewed every bit of this in depth yet, but it's
1:15am, so I'm going to post what I have and throw in the towel for
tonight.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
