2010/1/22 Robert Haas <robertmhaas@gmail.com>:
> On Fri, Jan 22, 2010 at 7:19 AM, Pavel Stehule <pavel.stehule@gmail.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.
Sure, I'll enhance comments. All \command is verified on broken
scanning. So when command's processing time the scanning is broken,
then we can detect it on the end - flag valid is changed only to false
direction. \set is different - it only one statement, that store
values. And it is reason why I did more checks there. I am not sure -
it is necessary now - simply we can set ERROR on the end and finish.
psql_scan_reset is called on the end of any statement - so we can
there reset info about scanning.
I'll look on it tomorrow.
Regards
Pavel Stehule
>
> 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
>