Andrew Dunstan wrote:
This bit:
> +/*
> + * Determine how we want to render values of a given type in datum_to_jsonb.
> + *
> + * Given the datatype OID, return its JsonbTypeCategory, as well as the type's
> + * output function OID. If the returned category is JSONBTYPE_CAST, we
> + * return the OID of the type->JSON cast function instead.
> + */
> +static void
> +jsonb_categorize_type(Oid typoid,
> + JsonbTypeCategory * tcategory,
> + Oid *outfuncoid)
> +{
seems like it can return without having set the category and func OID,
if there's no available cast. Callers don't seem to check for this
condition; is this a bug? If not, why not? Maybe some extra comments
are warranted.
Right now, for the "general case" there, there are two syscache lookups
rather than one. The fix is simple: just do the getTypeOutputInfo call
inside each case inside the switch instead of once at the beginning, so
that the general case can omit it; then there is just one syscache
access in all the cases. json.c suffers from the same problem.
Anyway this whole business of searching through the CASTSOURCETARGET
syscache seems like it could be refactored. If I'm counting correctly,
that block now appears four times (three in this patch, once in json.c).
Can't we add a new function to (say) lsyscache and remove that?
I'm just commenting on that part because the syscache.h/pg_cast.h
inclusions look a bit out of place; it's certainly not a serious issue.
I looked at what makes you include miscadmin.h. It's only USE_XSD_DATES
as far as I can tell. I looked at how that might be fixed, and a quick
patch that moves DateStyle, DateOrder and IntervalStyle (and associated
definitions) to datetime.h seems to work pretty well ... except that
initdb.c requires to know about some DATEORDER constants; but frontend
code cannot include datetime.h because of Datum. So that idea crashed
and burned until someone reorganizes the whole datetime code, which
currently is pretty messy.
I don't have any further comments on this patch, other than please add
JsonbTypeCategory to pgindent/typedefs.list before doing your pgindent
run.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
