Dilip Kumar <dilipbal...@gmail.com> writes: > On Thu, Sep 8, 2016 at 2:23 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: >> In the case of record_in, it seems like it'd be easy enough to use >> lookup_rowtype_tupdesc_noerror() instead of lookup_rowtype_tupdesc() >> and then throw a user-facing error right there.
> Actually when we are passing invalid type to "record_in", error is > thrown in "lookup_type_cache" function, and this function is not > taking "no_error" input (it's only limited to > lookup_rowtype_tupdesc_internal). Ah, should have dug a bit deeper. > One option is to pass "no_error" parameter to "lookup_type_cache" > function also, and throw error only in record_in function. > But problem is, "lookup_type_cache" has lot of references. And also > will it be good idea to throw one generic error instead of multiple > specific errors. ? We could make it work like that without breaking the ABI if we were to add a NOERROR bit to the allowed "flags". However, after looking around a bit I'm no longer convinced what I said above is a good idea. In particular, if we put the responsibility of reporting the error on callers then we'll lose the ability to distinguish "no such pg_type OID" from "type exists but it's only a shell". So I'm now thinking it's okay to promote lookup_type_cache's elog to a full ereport, especially since it's already using ereport for some of the related error cases such as the shell-type failure. That still leaves us with what to do for domain_in. A really simple fix would be to move the InitDomainConstraintRef call before the getBaseTypeAndTypmod call, because the former fetches the typcache entry and would then benefit from lookup_type_cache's ereport. But that seems a bit magic. I'm tempted to propose that domain_state_setup start with typentry = lookup_type_cache(domainType, TYPECACHE_DOMAIN_INFO); if (typentry->typtype != TYPTYPE_DOMAIN) ereport(ERROR, (errcode(ERRCODE_DATATYPE_MISMATCH), errmsg("type %s is not a domain", format_type_be(domainType)))); removing the need to verify that after getBaseTypeAndTypmod. The cache loading is basically free because InitDomainConstraintRef would do it anyway; so the extra cost here is only one dynahash search. You could imagine buying back those cycles by teaching the typcache to be able to cache the result of getBaseTypeAndTypmod, but I'm doubtful that we really care. This whole setup sequence only happens once per query anyway. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers