Re: Implicit coercions need to be reined in - Mailing list pgsql-hackers

Thomas Lockhart <lockhart@fourpalms.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