Re: BUG #15395: Assert failure when using CURRENT OF with inheritance

Sorry for the delayed response. The patch looks good to me and your
explanation makes sense.

I just re-ran my original test sql script on an official docker image of
postgres (which obviously has asserts off) and didn't get any errors + the
correct row is deleted.
I can investigate further if you think it's worthwhile.

Thanks for all your help.

-Mat

On Sun, Sep 23, 2018 at 4:06 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> I wrote:
> > I've not tried yet to create an actual test case for the chgParam-based
> > failure. It's definitely possible that that problem is only hypothetical
> > at the moment because cursor plans that would satisfy search_plan_tree
> > would be too simple to contain any such flags. But I think we'd better
> > add that logic anyway.
>
> After fooling with that for awhile longer, indeed there doesn't seem to
> be a way to cause such a failure today. The closest thing I was able
> to find was that an InitPlan's output params could get marked for
> recalculation, resulting in chgParam getting set on nodes that reference
> the InitPlan. A cursor query could certainly contain an InitPlan ---
> but there doesn't seem to be any code path that would result in forcing
> recalculation of one, except with things that couldn't be in a simple
> cursor plan, such as NestLoop joins. Still, it's not real hard to
> conceive of future changes that would render this a live issue, so
> I still feel that the search_plan_tree change is a good idea.
>
> So, I've added a test case based on your example and pushed the patch.
>
> BTW, I don't understand your report that just removing the Assert made
> it work for you. That would've resulted in returning an invalid TID
> to nodeTidscan.c, which AFAICS should've ended up with an ereport about
> a bad block number in heap_fetch.
>
> regards, tom lane
>