On Thu, Mar 8, 2018 at 3:01 PM, Amit Kapila <amit.kapil...@gmail.com> wrote: > On Thu, Mar 8, 2018 at 12:52 PM, Pavan Deolasee > <pavan.deola...@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 >>> <pavan.deola...@gmail.com> wrote: >>> > >>> > On Tue, Feb 13, 2018 at 12:41 PM, amul sul <sula...@gmail.com> wrote: >>> >> >>> >> Thanks for the confirmation, updated patch attached. >>> >> >>> > >>> > I am actually very surprised that 0001-Invalidate-ip_blkid-v5.patch does >>> > not >>> > do anything to deal with the fact that t_ctid may no longer point to >>> > itself >>> > to mark end of the chain. I just can't see how that would work. >>> > >>> >>> I think it is not that patch doesn't care about the end of the chain. >>> For example, ctid pointing to itself is used to ensure that for >>> deleted rows, nothing more needs to be done like below check in the >>> ExecUpdate/ExecDelete code path. >> >> >> Yeah, but it only looks for places where it needs to detect deleted tuples >> and thus wants to throw an error. I am worried about other places where it >> is assumed that the ctid points to a valid looking tid, self or otherwise. I >> see no such places being either updated or commented. >> >> Now may be there is no danger because of other protections in place, but it >> looks hazardous. >> > > Right, I feel we need some tests to prove it, I think as per code I > can see we need checks in few more places (like the ones mentioned in > the previous email) apart from where this patch has added. > >>> >>> >>> I have not tested, but it seems this could be problematic, but I feel >>> we could deal with such cases by checking invalid block id in the >>> above if check. Another one such case is in EvalPlanQualFetch >>> >> >> Right. >> > > Amul, can you please look into the scenario being discussed and see if > you can write a test to see the behavior. >
Sure, I'll try. Regards, Amu
