Re: Implicit coercions need to be reined in

Thomas Lockhart <lockhart(at)fourpalms(dot)org> writes:
> We have never been in complete agreement on the optimal behavior for
> type coersion, but it seems that most users are blissfully ignorant of
> the potential downsides of the current behavior. Another way to phrase
> that would be to say that it actually does the right thing in the vast
> majority of cases out in the field.

Could be; we probably see more complaints about the lack of any coercion
path for particular cases than about inappropriate implicit coercions.
But we do see a fair number of the latter. (And in the cases where I've
resisted adding more coercions, it was precisely because I thought it'd
be dangerous to allow them implicitly --- that concern goes away once
we can mark a coercion function as not implicitly invokable.)

> We'll probably both agree that it would be nice to avoid *hard coded*
> rules of any kind for this, but do you share my concern that moving this
> to a database table-driven set of rules will affect performance too
> much?

AFAICT the performance cost is negligible: find_coercion_function has to
look at the pg_proc row anyway. The relevant change looks like

PointerGetDatum(oid_array),
ObjectIdGetDatum(typnamespace));
! if (!HeapTupleIsValid(ftup))
! {
! ReleaseSysCache(targetType);
! return InvalidOid;
! }
! /* Make sure the function's result type is as expected, too */
! pform = (Form_pg_proc) GETSTRUCT(ftup);
! if (pform->prorettype != targetTypeId)
{
ReleaseSysCache(ftup);
- ReleaseSysCache(targetType);
- return InvalidOid;
}
! funcid = ftup->t_data->t_oid;
! ReleaseSysCache(ftup);
ReleaseSysCache(targetType);
return funcid;
}
--- 711,734 ----
Int16GetDatum(nargs),
PointerGetDatum(oid_array),
ObjectIdGetDatum(typnamespace));
! if (HeapTupleIsValid(ftup))
{
+ Form_pg_proc pform = (Form_pg_proc) GETSTRUCT(ftup);
+
+ /* Make sure the function's result type is as expected */
+ if (pform->prorettype == targetTypeId && !pform->proretset &&
+ !pform->proisagg)
+ {
+ /* If needed, make sure it can be invoked implicitly */
+ if (isExplicit || pform->proimplicit)
+ {
+ /* Okay to use it */
+ funcid = ftup->t_data->t_oid;
+ }
+ }
ReleaseSysCache(ftup);
}
!
ReleaseSysCache(targetType);
return funcid;
}

I do not see any reason not to install the mechanism; we can fine-tune
the actual pg_class.proimplicit settings as we get experience with them.

regards, tom lane