On Thu, Nov 16, 2017 at 9:37 AM, amul sul <sulamul@gmail.com> wrote:
> Fixed in the 001 patch.
>
> IMHO, this function is not meant for a user, so that instead of ereport() cant
> we simply return false? TWIW, I have attached 003 patch which replaces all
> erepots() by return false.
I don't think just returning false is very helpful behavior, because
the user may not realize that the problem is that the function is
being called incorrectly rather than that the call is correct and the
answer is false.
I took your 0001 patch and made extensive modifications to it. I
replaced your regression tests from 0002 with a new set that I wrote
myself. The result is attached here. This version makes different
decisions about how to handle the various problem cases than you did;
it returns NULL for a NULL input or an OID for which no relation
exists, and throws specific error messages for the other cases,
matching the parser error messages that CREATE TABLE would issue where
possible, but with a different error code. It also checks that the
types match (which your patch did not, and which I'm fairly sure could
crash the server), makes the function work when invoked using the
explicit VARIADIC syntax (which seems fairly useless here but there's
no in-core precedent for variadic function which doesn't support that
case), and fixes the function header comment to describe the behavior
we committed rather than the older behavior you had in earlier patch
versions.
As far as I can tell, this should nail things down pretty tight, but
it would be great if someone can try to find a case where it still
breaks.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company