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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers