On 8/2/07, Heikki Linnakangas <[EMAIL PROTECTED]> wrote: > > Pavan Deolasee wrote: > > Please see the attached version 11 of HOT patch > > Thanks! > > One wrinkle in the patch is how the ResultRelInfo-struct is passed to > heap_update, and on to heap_check_idxupdate, to check any indexed > columns have changed. I think that's a modularity violation, heap_update > really shouldn't have to deal with ResultRelInfo, that belongs in the > executor. When we add support for expression and partial indexes, > heap_check_idxupdate will need even more executor machinery to be able > to evaluate expressions.
The reason I put it there because we wanted to do that check as late as possible, once we confirm that update is possible and there is enough space in the block to perform HOT update. But I agree thats a modularity violation. Any suggestion to avoid that ? In heap_page_prune_defrag, it would be better to do the test for > BufferIsLockedForCleanup right after acquiring the lock. The longer the > delay between those steps, the bigger the chances that someone pins the > page and starts to wait for the buffer lock, making us think that we > didn't get the cleanup lock, though we actually did. Maybe a nicer > solution would be to have another version of ConditionalLockBuffer with > three different return values: didn't get lock, got exclusive lock, or > got cleanup lock. Thats a good idea. I shall do that. It's not necessary to WAL-log the unused-array that > PageRepairFragmentation returns. In replay, a call to > PageRepairFragmentation will come to the same conclusion about which > line pointers are not used. It would also be better if we didn't emit a > separate WAL record for defraging a page, if we also prune it at the > same time. I'm not that worried about WAL usage in general, but that > seems simple enough to fix. Ah I see. I shall fix that. Thanks, Pavan -- Pavan Deolasee EnterpriseDB http://www.enterprisedb.com
