In the interest of moving forward, I have updated this patch because
Ryan has been inactive for over a month now.
Tom Lane wrote:
> Yeah, setup_parser_errposition_callback would work too. I'm not sure
> offhand which I like better. One thing to keep in mind is that the
> callback approach results in adding an error cursor position to *any*
> error thrown while the callback is active, not only "schema not found".
> There are pluses and minuses to that. I've seen error cursors attached
> to very bizarre internal problems that (more or less by chance) showed up
> while the parser was looking up a table name, but weren't really connected
> to the table name at all. OTOH, most of the time you'd just as soon not
> be too picky about what conditions you provide a cursor for.
I think we can live with cursor positions in some weird corner cases.
If we later find out that we don't like it for some reason, we can
reduce the scope that this applies to.
> The main place I'd question what you did is the callback placement around
> make_oper_cache_key --- that might work, but it seems very bizarre, and
> perhaps more than usually prone to the "cursor given for unrelated
> problem" issue. Perhaps better to push it down inside that function
> so it surrounds just the namespace lookup call.
Agreed; the attached patch does it that way. (I notice that we have the
pstate as first arg in many places; I put at the end for
make_oper_cache_key, together with location. Is there some convention
to have it as first arg?)
> Also the diffs in parse_utilcmd.c are very confusing and seem to change
> more code than is necessary --- why did that happen?
The reason appears to be that Ryan wanted to have the pstate set, but
that was only created after looking other things up, so he moved a
largish block down; this was pretty bogus AFAICT. The attached patch
fixes this by first creating the pstate, then doing the namespace
lookup, then doing the rest of the setup.
It's a bit disappointing to see so little changes in regression expected
output ...
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services