On Tue, Dec 3, 2013 at 8:18 AM, Andres Freund <andres@2ndquadrant.com> wrote:
>> 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.
>
> Hm. I think it actually will not abort early in call cases either, but
> that looks fixable. Imagine what happens if id_attrs or key_attrs is
> empty, ISTM that we'll check all hot columns in that case.
Yeah, you're right. I think the current logic will terminate when all
flags are set to false or all attribute numbers have been checked, but
it doesn't know that if HOT's been disproven then we needn't consider
further HOT columns. I think the way to fix that is to tweak this
part:
+ if (next_hot_attnum > FirstLowInvalidHeapAttributeNumber) check_now =
next_hot_attnum;
+ else if (next_key_attnum > FirstLowInvalidHeapAttributeNumber)
+ check_now = next_key_attnum;
+ else if (next_id_attnum > FirstLowInvalidHeapAttributeNumber)
+ check_now = next_id_attnum; else
+ break;
What I think we ought to do there is change each of those criteria to
say if (hot_result && next_hot_attnum >
FirstLowInvalidHeapAttributeNumber) and similarly for the other two.
That way we consider each set a valid source of attribute numbers only
until the result flag for that set flips false.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company