Re: quoting psql varible as identifier

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

In response to

Responses

Browse pgsql-hackers by date

  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]