Re: WIP: pl/pgsql cleanup

Neil Conway <neilc@samurai.com> writes:
> Attached is a revised patch that removes "embedded" and updates the
> regression tests. I'll apply this to HEAD later today barring any
> further suggestions for improvement.

You've broken the FOR syntax.  You may not assume that the first token
of a FOR-over-SELECT is literally SELECT; it could for example be a left
parenthesis starting some kind of UNION construct.  This is why it's so
hard to do it right, and why the old way was so messy.  (As of CVS tip
it also works to do something like
    for x in explain analyze select ...
I will grant that that didn't work before today, but it wasn't plpgsql's
fault that it didn't.)

I suggest you go back to the old parsing code for FOR.

I think it's a bad idea to entirely override the error context stack as
you do in check_sql_expr().  I can see the case for removing the
immediately previous entry, if you're sure it is
plpgsql_compile_error_callback(), but that doesn't translate to it being
a good idea to knock out any surrounding levels of context.

(Also I thought you were going to reword the "embedded" message?)

The head comment added to do_compile could stand some copy-editing :-(

Otherwise it looks pretty good...

            regards, tom lane