Re: Add LINE: hint when schemaname.typename is a non-existent schema

Ryan Kelly <rpkelly22(at)gmail(dot)com> writes:
> On Tue, Feb 3, 2015 at 11:10 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> It's probably not reasonable to add a pstate argument to
>> LookupExplicitNamespace itself, given that namespace.c isn't part
>> of the parser. But you could imagine adding an interface function
>> in the parser that calls LookupExplicitNamespace and then throws a
>> position-aware error on failure, and then making sure that all schema
>> lookups in the parser go through that.

> While searching for other instances of this problem, I found that using
> a schema that does not exist in conjunction with a collation reported the
> position. That code uses setup_parser_errposition_callback and the
> documentation for that function seems to indicate that the usage of it in my
> newly attached patch are consistent. I do not know, however, if this is the
> cleanest approach or if I should actually create a function like
> validate_explicit_namespace to handle this (and I'm also not sure where
> such a function should live, if I do create it).

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.

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.

Also the diffs in parse_utilcmd.c are very confusing and seem to change
more code than is necessary --- why did that happen?

regards, tom lane