2010/1/28 Robert Haas <robertmhaas@gmail.com>:
> On Thu, Jan 28, 2010 at 4:53 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> 2010/1/27 Robert Haas <robertmhaas@gmail.com>:
>>> On Mon, Jan 25, 2010 at 7:36 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>>>> I hope, so this version is more readable and more clean. I removed
>>>> some not necessary checks.
>>>
>>> This still seems overly complicated to me. I spent a few hours today
>>> working up the attached patch. Let me know your thoughts.
>>
>> There is serious issue. The "psql_error" only shows some message on
>> output, but do nothing more - you don't set a result status for
>> commands and for statements. So some potential error from parsing is
>> pseudo quietly ignored - without respect to your setting
>> ON_ERROR_STOP. This could be a problem for commands. Execution of
>> broken SQL statements will raise syntax error. But for \set some
>> variable will be a broken and the content can be used. I don't thing
>> so it is good. It is limited.
>
> Well, what you seem to be proposing to do is allow the command to
> execute (on the screwed-up data) and then afterwards pretend that it
> failed by overriding the return status. I think that's unacceptable.
> The root of the problem here is that the parsing and execution stages
> for backslash commands are not cleanly separated. There's no clean
> way for the lexer to return an error that allows the command to finish
> parsing normally but then doesn't execute it. Fixing that is going to
> require an extensive refactoring of commands.c which I don't think it
> makes sense to undertake at this point in the release cycle. Even if
> it did, it seems like material for a separate patch rather than
> something which has to be done before this goes in.
so I removed support for escaping from backslah commands and refactorised code.
I hope so this code is more verbose and clean than previous versions.
Regards
Pavel
>
>> Your version is acceptable only when we don't enable escape syntax for
>> commands. Then we don't need check it. On your version - I am not sure
>> if it is fully compatible, and using a global variables isn't nice.
>
> I'm not adding any new global variables - I'm just using the ones that
> are already there to avoid duplicating the same code four times.
> Referencing them from within the bodies of the lexer rules doesn't
> make the variables not global.
>
> ...Robert
>
