Re: BUG #11335: an invalid prepare statement causes crash at log_statement = 'mod' or 'ddl'.

On Sat, Sep 6, 2014 at 1:34 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2014-09-06 16:25:28 -0400, Tom Lane wrote:
>> Fujii Masao <masao(dot)fujii(at)gmail(dot)com> writes:
>> > On Thu, Sep 4, 2014 at 3:55 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> >> Thanks for reporting the bug! This segmentation fault could reproduce
>> >> even on my machine. Barring any objection, I will apply the change that
>> >> you suggested.
>>
>> > Done.
>>
>> I don't think this fix is either appropriate or adequate.
>
> Agreed (and commented offlist. Which probably was a mistake).
This has not been reverted yet. Wouldn't it be better to do that asap?
This would improve chances of seeing any potential issues in this code
path if it gets broken once again.

>> Or maybe we should reconsider whether
>> exec_parse_message should allow the case at all. It's not unreasonable
>> that Parse should require exactly one SQL statement, not just "at most
>> one".
>
> I think this is the best bet. There really doesn't seem much
> justification for the current "at most one" definition. At least not one
> that I could find or think of. The likelihood of leaving some places
> unfixed or new breakages creeping in seems non-nil; it's not what one
> would immediately expect.
Looking at exec_parse_message, empty input string is allowed for a
cached plan (16503e6f). This solution would break client
applications/drivers using the extending query protocol and relying on
this behavior. This EmptyStmt approach sounds like a good option.
--
Michael