On Tue, Mar 28, 2017 at 4:05 PM, Amit Kapila <amit.kapil...@gmail.com> wrote:
> > > As asked previously, can you explain me on what basis are you > considering it robust? The comments on top of datumIsEqual() clearly > indicates the danger of using it for toasted values (Also, it will > probably not give the answer you want if either datum has been > "toasted".). Hmm. I don' see why the new code in recheck is unsafe. The index values themselves can't be toasted (IIUC), but they can be compressed. index_form_tuple() already untoasts any toasted heap attributes and compresses them if needed. So once we pass heap values via index_form_tuple() we should have exactly the same index values as they were inserted. Or am I missing something obvious here? If you think that because we are using it during > heap_update to find modified columns, then I think that is not right > comparison, because there we are comparing compressed value (of old > tuple) with uncompressed value (of new tuple) which should always give > result as false. > > Hmm, this seems like a problem. While HOT could tolerate occasional false results (i.e. reporting a heap column as modified even though it it not), WARM assumes that if the heap has reported different values, then they better be different and should better result in different index values. Because that's how recheck later works. Since index expressions are not supported, I wonder if toasted heap values are the only remaining problem in this area. So heap_tuple_attr_equals() should first detoast the heap values and then do the comparison. I already have a test case that fails for this reason, so let me try this approach. Thanks, Pavan -- Pavan Deolasee http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
