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.
> 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
