Re: Segfault in backend CTE code

I screwed up cut and paste when putting the test case together. The
reference to table user_data should be t1.

On Wed, Jan 25, 2012 at 12:47 PM, Phil Sorber <phil@omniti.com> wrote:
> On Tue, Jan 24, 2012 at 4:03 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Phil Sorber <phil@omniti.com> writes:
>>> On Tue, Jan 24, 2012 at 12:43 AM, Tom Lane <tgl@sss.pgh.pa.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