From: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: quoting psql varible as identifier |
Date: | 2010-01-28 16:59:32 |
Message-ID: | 162867791001280859u71c8540fu948bc4aea06249b9@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
2010/1/28 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Thu, Jan 28, 2010 at 4:53 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>> 2010/1/27 Robert Haas <robertmhaas(at)gmail(dot)com>:
>>> On Mon, Jan 25, 2010 at 7:36 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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
>
Attachment | Content-Type | Size |
---|---|---|
variable-escaping-simple.patch | text/x-patch | 10.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Andrew Dunstan | 2010-01-28 17:02:49 | Re: Add on_trusted_init and on_untrusted_init to plperl [PATCH] |
Previous Message | Tom Lane | 2010-01-28 16:54:08 | Re: Add on_trusted_init and on_untrusted_init to plperl [PATCH] |