On 11/04/2014 01:45 PM, Tom Lane wrote:
Andrew Dunstan <and...@dunslane.net> writes:
here's a patch for a utility function to look up the cast function for
a from/to pair of types, as recently suggested by Alvaro. Although it
only contains one use (in json.c), the upcoming jsonb generators would
also use it twice. I'd like to get this committed fairly quickly so I
can prepare an updated patch for the jsonb generators.
I'm not exactly convinced that this is an appropriate utility function,
because casts for which a COERCION_METHOD_FUNCTION entry exists in pg_cast
are by no means the whole universe of casts.  I'm concerned that creating
this function will encourage patch authors to blow off other
possibilities when they should not.  In the particular context at hand,
it seems like you might be better advised to think about how to refactor
json_categorize_type to be more helpful to your new use-case.

A concrete example of what I'm worried about is that json_categorize_type
already ignores the possibility of binary-compatible (WITHOUT FUNCTION)
casts to json.  Since it's eliminated the domain case earlier, that's
perhaps not too horridly broken; but it'd be very easy for new uses of
this get_cast_func() function to overlook such considerations.

In short, I'd rather see this addressed through functions with slightly
higher-level APIs that are capable of covering more cases.  In most cases
it'd be best if callers were using find_coercion_pathway() rather than
taking shortcuts.



Well, then, do we really need a wrapper at all? Should we just be doing something like this?

                if (typoid >= FirstNormalObjectId)
                {
                    Oid castfunc;
                    CoercionPathType ctype;

                    ctype = find_coercion_pathway(JSONOID, typoid,
                                                  COERCION_EXPLICIT, &castfunc);

                    if (ctype == COERCION_PATH_FUNC && OidIsValid(castfunc))
                    {
                        *tcategory = JSONTYPE_CAST;
                        *outfuncoid = castfunc;
                    }
                }



cheers

andrew


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to