From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Pavel Stehule <pavel(dot)stehule(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-22 19:20:01 |
Message-ID: | 603c8f071001221120ra2b6c7cne5105e8882960f18@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Jan 22, 2010 at 7:19 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
> here is new variant. Add scan_state flag "valid" and enhance
> protection against execution broken statement.
This doesn't make sense to me. You've changed the way \set is handled
- in a way that doesn't seem particularly appropriate to me - but most
of the other backslash commands are unmodified - but then there's this
hack at the bottom that overrides the return value if
psql_scan_is_valid() fails. And then there's this:
- /* we need not do psql_scan_reset() here */
+ psql_scan_reset(scan_state);
It's extremely unclear what the rationale for this change is.
Basically, you need to either improve the comments in here so that
someone can understand what is going on, or you need to submit it with
some detailed documentation explaining the rationale behind each
change and why it is right, or more than likely both. But I think the
whole approach is likely off-base and you need to go back and think
about a cleaner way to get this done.
...Robert
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2010-01-22 20:09:29 | Re: review: More frame options in window functions |
Previous Message | David E. Wheeler | 2010-01-22 19:14:12 | Review: listagg aggregate |