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. If we're going to keep it, it
should have a test. 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.
-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).
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.
--
Robert Haas
EDB: http://www.enterprisedb.com
