Re: Identifying function-lookup failures due to argument name mismatches

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> Some review of 0001:
> + return errdetail("A procedure of that name exists, but it is not in
> the search_path.");

> Are you sure you want to expose this case in this way? From a security
> point of view it makes me a bit nervous.

I'm not seeing the security concern? An attacker who can issue a
SQL command that would trigger this could presumably also issue a
"SELECT FROM pg_proc" that would reveal the same info and more.

> If we're going to keep it, it
> should have a test.

Huh, I thought I had covered the case, but you're right it's not
anywhere in the .out files. Will fix.

> Even from a non-security perspective, maybe having
> the error message vary based on the contents of a completely unrelated
> schema is not the best design decision. I can imagine that hosing some
> user that is looking for a specific message and then somebody installs
> an extension and the message changes even though there's no reason for
> them to interact.

The primary error message is not varying, only the DETAIL/HINT, so
I find this concern pretty far-fetched. Also, I believe that the
case that the message intends to help with is very common and so
it will save a lot of people time, more than enough to outweigh
any cases where it's perhaps un-optimal.

> -HINT: No function matches the given name and argument types. You
> might need to add explicit type casts.

> I wonder what caused this line to disappear without being replaced by
> anything (test_extensions.out).

That is the response to

ERROR: function public.dep_req2() does not exist
LINE 1: SELECT public.dep_req2() || ' req3b'
^
-HINT: No function matches the given name and argument types. You might need to add explicit type casts.

and I omitted the hint/detail because it seems to add nothing,
per this argument:

+ * If not FGC_NAME_VISIBLE, we shouldn't raise the question of whether the
+ * arguments are wrong. If the function name was not schema-qualified,
+ * it's helpful to distinguish between doesn't-exist-anywhere and
+ * not-in-search-path; but if it was, there's really nothing to add to the
+ * basic "function/procedure %s does not exist" message.

I'm certainly willing to discuss that choice, but I wonder what you
have in mind that isn't just a restatement of "function does not
exist". There flat out isn't a pg_proc entry of the name that
the user gave. We could issue something like "HINT: maybe you
misspelled the function name." or "HINT: maybe there's some extension
you need to install." but that's getting way too nanny-ish for my
taste. The odds of giving an on-point hint don't seem good.

> Overall, I like this. I think these changes are helpful.
> Some review of 0002:
> I don't mind the churn. It is perhaps not mandatory, though. Call me +0.5.
> Comments above also basically apply to 0003 and 0004: not mandatory, I
> tentatively think they're improvements, be careful about the
> not-in-schema case, test it if we're going to expose that information.

Fair enough.

regards, tom lane