Re: Segfault in backend CTE code

On Tue, Jan 24, 2012 at 4:03 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Phil Sorber <phil(at)omniti(dot)com> writes:
>> On Tue, Jan 24, 2012 at 12:43 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> How about a test case?
>
>> We are having trouble coming up with a test case that can reliably
>> reproduce this.
>
> The stack traces run through the EvalPlanQual machinery, which is only
> going to be entered when attempting to update/delete a row that's been
> updated by a concurrent transaction.  So what I'd suggest for creating a
> test case is
>
>        (1) in a psql session, do
>                BEGIN;
>                UPDATE some-target-row;
>
>        (2) in another psql session, call this function with arguments
>            that will make it try to update that same row; it should
>            block.
>
>        (3) in the first session, COMMIT to unblock.
>

That helped a lot. I now have a simple test case that I can reliably
re-produce the segfault and now also a patch that fixes it. I had to
modify the patch slightly because while it fixed the first problem, it
just cascaded to another NULL deref from the same root cause. Both are
attached.

> FWIW, having re-examined your patch with some caffeine in me, I don't
> think it's right at all.  You can't just blow off setting the scan type
> for a CTEScan node.  What it looks like to me is that the EvalPlanQual
> code is not initializing the new execution tree correctly; but that
> idea would be a lot easier to check into with a test case.
>

If I understand what you are saying, then I agree that is the root of
the problem. The comment label's it as an optimization, but then later
fails to account for all the changes needed. My patch accounts for at
least one extra change that is needed. We could also remove the
"optimization" but I assumed it was there for a reason, especially
given the fact that someone took the time to make a comment about it.

The change was made in this commit by you:
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;f=src/backend/executor/execMain.c;h=389af951552ff2209eae3e62fa147fef12329d4f

>                        regards, tom lane