On Thu, 2005-02-10 at 20:48 -0500, Tom Lane wrote:
> 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.
Yeah, I was wondering if this would break anything, I just forgot to ask
about it :( Looking for two periods is pretty ugly. I was thinking we
might be able to look at the for loop variable: if it was previously
undeclared, it must be an integer for loop. If it was declared but is
not of a row or record type, it must also be an integer for loop. But
this is still ambiguous in the case of a declared row or record type --
it could either be a SELECT loop or an integer loop, and in the latter
case the loop variable would shadow the variable in the enclosing
block :(
Unless we can figure out a better way to do this, I'll just revert to
the old kludge.
> 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.
Yes, that's a good point. I'll change the patch to just elide the
previous entry from the stack of callbacks, which is going to be
plpgsql_compile_error_callback (unfortunately we can't actually verify
that, AFAICS, since that callback is private to pl_comp.c)
-Neil
