On Fri, Mar 9, 2018 at 3:18 PM, amul sul <sula...@gmail.com> wrote:
> On Thu, Mar 8, 2018 at 12:31 PM, Amit Kapila <amit.kapil...@gmail.com> wrote:
>> On Thu, Mar 8, 2018 at 11:04 AM, Pavan Deolasee
>>> This is just one example. I am almost certain there are many such cases that
>>> will require careful attention.
>> Right, I think we should be able to detect and fix such cases.
> I found a couple of places (in heap_lock_updated_tuple, rewrite_heap_tuple,
> EvalPlanQualFetch & heap_lock_updated_tuple_rec) where ItemPointerEquals is
> use to check tuple has been updated/deleted. With the proposed patch
> ItemPointerEquals() will no longer work as before, we required addition check
> for updated/deleted tuple, proposed the same in latest patch. Do let me
> your thoughts/suggestions on this, thanks.
I think you have identified the places correctly. I have few
- if (!ItemPointerEquals(&tuple->t_self, ctid))
+ if (!(ItemPointerEquals(&tuple->t_self, ctid) ||
+ (!ItemPointerValidBlockNumber(ctid) &&
+ (ItemPointerGetOffsetNumber(&tuple->t_self) == /* TODO: Condn.
should be macro? */
Can't we write this and similar tests as:
!ItemPointerEquals(&tuple->t_self, ctid)? It will be bit simpler to
understand and serve the purpose.
if (mytup.t_data->t_infomask & HEAP_XMAX_INVALID ||
ItemPointerEquals(&mytup.t_self, &mytup.t_data->t_ctid) ||
+ !HeapTupleHeaderValidBlockNumber(mytup.t_data) ||
I think it is better to keep the check for
HeapTupleHeaderValidBlockNumber earlier than ItemPointerEquals as it
will first validate if block number is valid and then only compare the