Dilip Kumar <dilipbalaut@gmail.com> writes:
> Basically this patch changes error at three places.
> 1. getBaseTypeAndTypmod: This is being called from domain_in exposed
> function (domain_in->
> domain_state_setup-> getBaseTypeAndTypmod). Though this function is
> being called from many other places which are not exposed function,
> but I don't this will have any imapct, will this ?
I really don't like this solution. Changing this one function, out of the
dozens just like it in lsyscache.c, seems utterly random and arbitrary.
I'm actually not especially convinced that passing domain_in a random
OID needs to not produce an XX000 error. That isn't a user-facing
function and people shouldn't be calling it by hand at all. The same
goes for record_in. But if we do want those cases to avoid this,
I think it's appropriate to fix it in those functions, not decree
that essentially-random other parts of the system should bear the
responsibility. (Thought experiment: if we changed the order of
operations in domain_in so that getBaseTypeAndTypmod were no longer
the first place to fail, would we undo this change and then change
wherever the failure had moved to? That's pretty messy.)
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. Not sure about a
nice solution for domain_in. We might have to resort to an extra
syscache lookup there, but even if we did, it should happen only
once per query so that's not very awful.
> 3. CheckFunctionValidatorAccess: This is being called from all
> language validator functions.
This part seems reasonable, since the validator functions are documented
as something users might call, and CheckFunctionValidatorAccess seems
like an apropos place to handle it.
regards, tom lane
