From 44efba4a1d4d48ed533c5a26b454c2dd7b6aa433 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Tue, 30 Jun 2026 13:46:43 -0400 Subject: [PATCH v1 1/5] Refactor json_categorize_type() to populate an FmgrInfo * By doing this, any code that caches the results of json_categorize_type now caches the FmgrInfo * rather than just the function OID, which saves a modest number of CPU cycles. This affects all of the built-in JSON aggregates (json_agg, json_agg_strict, json_object_agg, jsonb_agg, etc.), a few plain functions like array_to_json(), and the JSON_SCALAR constructor. In my testing, the performance gains are small enough that they are difficult to measure at the SQL level, but doing this much allows for future refactorings with more significant benefits. As far as this refactoring goes, the most problematic caller of json_categorize_type() was json_check_mutability(), which doesn't actually need an FmgrInfo *. Therefore, I refactored json_check_mutability() to no longer call json_categorize_type(). Instead, it does the same work directly, except that a little bit of common logic has been factored out into a new function get_json_cast_for_type(). This refactoring also made it evident that json_check_mutability's is_jsonb flag has never had any effect on the behavior, so it is removed. Since this is intended as a refactoring commit only, it attempts to avoid changing the behavior of the code even when that seems to have previously been incorrect. In the previous coding, json_check_mutability failed to notice that the output functions for anyarray, anycompatiblearray, and record are not immutable. Other container types were determined to be non-immutable by recursing to their element or base types, but these are implemented as pseudotypes, so the recursive code does not realize that they can have arbitrary types as substructure. For now, a special case is added to preserve the historical behavior, but in a separate commit we should remove the special case to obtain more correct behavior. --- src/backend/executor/execExpr.c | 5 +- src/backend/executor/execExprInterp.c | 6 +- src/backend/utils/adt/json.c | 125 +++++++++++--------- src/backend/utils/adt/jsonb.c | 109 +++++++++-------- src/backend/utils/adt/jsonfuncs.c | 162 ++++++++++++++------------ src/include/executor/execExpr.h | 2 +- src/include/utils/jsonfuncs.h | 10 +- src/test/regress/expected/sqljson.out | 4 + src/test/regress/sql/sqljson.sql | 5 + 9 files changed, 235 insertions(+), 193 deletions(-) diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index cfea7e160c2..871fd17e1ca 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -2437,13 +2437,12 @@ ExecInitExprRec(Expr *node, ExprState *state, for (int i = 0; i < nargs; i++) { JsonTypeCategory category; - Oid outfuncid; Oid typid = jcstate->arg_types[i]; json_categorize_type(typid, is_jsonb, - &category, &outfuncid); + &category, + &jcstate->arg_type_cache[i].outflinfo); - jcstate->arg_type_cache[i].outfuncid = outfuncid; jcstate->arg_type_cache[i].category = (int) category; } } diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index 0634af964a9..1ebfb190e67 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -4766,14 +4766,14 @@ ExecEvalJsonConstructor(ExprState *state, ExprEvalStep *op, else { Datum value = jcstate->arg_values[0]; - Oid outfuncid = jcstate->arg_type_cache[0].outfuncid; + FmgrInfo *outflinfo = &jcstate->arg_type_cache[0].outflinfo; JsonTypeCategory category = (JsonTypeCategory) jcstate->arg_type_cache[0].category; if (is_jsonb) - res = datum_to_jsonb(value, category, outfuncid); + res = datum_to_jsonb(value, category, outflinfo); else - res = datum_to_json(value, category, outfuncid); + res = datum_to_json(value, category, outflinfo); } } else if (ctor->type == JSCTOR_JSON_PARSE) diff --git a/src/backend/utils/adt/json.c b/src/backend/utils/adt/json.c index 0fee1b40d63..788237f6b48 100644 --- a/src/backend/utils/adt/json.c +++ b/src/backend/utils/adt/json.c @@ -80,21 +80,21 @@ typedef struct JsonAggState { StringInfo str; JsonTypeCategory key_category; - Oid key_output_func; + FmgrInfo key_flinfo; JsonTypeCategory val_category; - Oid val_output_func; + FmgrInfo val_flinfo; JsonUniqueBuilderState unique_check; } JsonAggState; static void array_dim_to_json(StringInfo result, int dim, int ndims, int *dims, const Datum *vals, const bool *nulls, int *valcount, - JsonTypeCategory tcategory, Oid outfuncoid, + JsonTypeCategory tcategory, FmgrInfo *flinfo, bool use_line_feeds); static void array_to_json_internal(Datum array, StringInfo result, bool use_line_feeds); static void datum_to_json_internal(Datum val, bool is_null, StringInfo result, - JsonTypeCategory tcategory, Oid outfuncoid, - bool key_scalar); + JsonTypeCategory tcategory, + FmgrInfo *outflinfo, bool key_scalar); static void add_json(Datum val, bool is_null, StringInfo result, Oid val_type, bool key_scalar); static text *catenate_stringinfo_string(StringInfo buffer, const char *addon); @@ -168,15 +168,19 @@ json_recv(PG_FUNCTION_ARGS) /* * Turn a Datum into JSON text, appending the string to "result". * - * tcategory and outfuncoid are from a previous call to json_categorize_type, - * except that if is_null is true then they can be invalid. + * tcategory is from a previous call to json_categorize_type, except that if + * is_null is true then it can be invalid. + * + * outflinfo is a pointer to an FmgrInfo populated by the same call to + * json_categorize_type, but it is only needed for categories where + * json_categorize_type actually populates it. * * If key_scalar is true, the value is being printed as a key, so insist * it's of an acceptable type, and force it to be quoted. */ static void datum_to_json_internal(Datum val, bool is_null, StringInfo result, - JsonTypeCategory tcategory, Oid outfuncoid, + JsonTypeCategory tcategory, FmgrInfo *outflinfo, bool key_scalar) { char *outputstr; @@ -221,7 +225,7 @@ datum_to_json_internal(Datum val, bool is_null, StringInfo result, appendStringInfoChar(result, '"'); break; case JSONTYPE_NUMERIC: - outputstr = OidOutputFunctionCall(outfuncoid, val); + outputstr = OutputFunctionCall(outflinfo, val); /* * Don't quote a non-key if it's a valid JSON number (i.e., not @@ -274,25 +278,26 @@ datum_to_json_internal(Datum val, bool is_null, StringInfo result, break; case JSONTYPE_JSON: /* JSON and JSONB output will already be escaped */ - outputstr = OidOutputFunctionCall(outfuncoid, val); + outputstr = OutputFunctionCall(outflinfo, val); appendStringInfoString(result, outputstr); pfree(outputstr); break; case JSONTYPE_CAST: - /* outfuncoid refers to a cast function, not an output function */ - jsontext = DatumGetTextPP(OidFunctionCall1(outfuncoid, val)); + /* outflinfo refers to a cast function, not an output function */ + jsontext = DatumGetTextPP(FunctionCall1(outflinfo, val)); appendBinaryStringInfo(result, VARDATA_ANY(jsontext), VARSIZE_ANY_EXHDR(jsontext)); pfree(jsontext); break; default: /* special-case text types to save useless palloc/memcpy cycles */ - if (outfuncoid == F_TEXTOUT || outfuncoid == F_VARCHAROUT || - outfuncoid == F_BPCHAROUT) + if (outflinfo->fn_oid == F_TEXTOUT || + outflinfo->fn_oid == F_VARCHAROUT || + outflinfo->fn_oid == F_BPCHAROUT) escape_json_text(result, (text *) DatumGetPointer(val)); else { - outputstr = OidOutputFunctionCall(outfuncoid, val); + outputstr = OutputFunctionCall(outflinfo, val); escape_json(result, outputstr); pfree(outputstr); } @@ -429,7 +434,7 @@ JsonEncodeDateTime(char *buf, Datum value, Oid typid, const int *tzp) static void array_dim_to_json(StringInfo result, int dim, int ndims, int *dims, const Datum *vals, const bool *nulls, int *valcount, JsonTypeCategory tcategory, - Oid outfuncoid, bool use_line_feeds) + FmgrInfo *outflinfo, bool use_line_feeds) { int i; const char *sep; @@ -448,8 +453,7 @@ array_dim_to_json(StringInfo result, int dim, int ndims, int *dims, const Datum if (dim + 1 == ndims) { datum_to_json_internal(vals[*valcount], nulls[*valcount], - result, tcategory, - outfuncoid, false); + result, tcategory, outflinfo, false); (*valcount)++; } else @@ -459,7 +463,7 @@ array_dim_to_json(StringInfo result, int dim, int ndims, int *dims, const Datum * we'll say no. */ array_dim_to_json(result, dim + 1, ndims, dims, vals, nulls, - valcount, tcategory, outfuncoid, false); + valcount, tcategory, outflinfo, false); } } @@ -484,7 +488,7 @@ array_to_json_internal(Datum array, StringInfo result, bool use_line_feeds) bool typbyval; char typalign; JsonTypeCategory tcategory; - Oid outfuncoid; + FmgrInfo outflinfo; ndim = ARR_NDIM(v); dim = ARR_DIMS(v); @@ -500,14 +504,14 @@ array_to_json_internal(Datum array, StringInfo result, bool use_line_feeds) &typlen, &typbyval, &typalign); json_categorize_type(element_type, false, - &tcategory, &outfuncoid); + &tcategory, &outflinfo); deconstruct_array(v, element_type, typlen, typbyval, typalign, &elements, &nulls, &nitems); array_dim_to_json(result, 0, ndim, dim, elements, nulls, &count, tcategory, - outfuncoid, use_line_feeds); + &outflinfo, use_line_feeds); pfree(elements); pfree(nulls); @@ -558,7 +562,7 @@ composite_to_json(Datum composite, StringInfo result, bool use_line_feeds) bool isnull; char *attname; JsonTypeCategory tcategory; - Oid outfuncoid; + FmgrInfo outflinfo; Form_pg_attribute att = TupleDescAttr(tupdesc, i); if (att->attisdropped) @@ -575,16 +579,13 @@ composite_to_json(Datum composite, StringInfo result, bool use_line_feeds) val = heap_getattr(tuple, i + 1, tupdesc, &isnull); if (isnull) - { tcategory = JSONTYPE_NULL; - outfuncoid = InvalidOid; - } else json_categorize_type(att->atttypid, false, &tcategory, - &outfuncoid); + &outflinfo); - datum_to_json_internal(val, isnull, result, tcategory, outfuncoid, - false); + datum_to_json_internal(val, isnull, result, + tcategory, &outflinfo, false); } appendStringInfoChar(result, '}'); @@ -603,7 +604,7 @@ add_json(Datum val, bool is_null, StringInfo result, Oid val_type, bool key_scalar) { JsonTypeCategory tcategory; - Oid outfuncoid; + FmgrInfo outflinfo; if (val_type == InvalidOid) ereport(ERROR, @@ -611,15 +612,12 @@ add_json(Datum val, bool is_null, StringInfo result, errmsg("could not determine input data type"))); if (is_null) - { tcategory = JSONTYPE_NULL; - outfuncoid = InvalidOid; - } else json_categorize_type(val_type, false, - &tcategory, &outfuncoid); + &tcategory, &outflinfo); - datum_to_json_internal(val, is_null, result, tcategory, outfuncoid, + datum_to_json_internal(val, is_null, result, tcategory, &outflinfo, key_scalar); } @@ -697,7 +695,7 @@ to_json_is_immutable(Oid typoid) { bool has_mutable = false; - json_check_mutability(typoid, false, &has_mutable); + json_check_mutability(typoid, &has_mutable); return !has_mutable; } @@ -710,7 +708,7 @@ to_json(PG_FUNCTION_ARGS) Datum val = PG_GETARG_DATUM(0); Oid val_type = get_fn_expr_argtype(fcinfo->flinfo, 0); JsonTypeCategory tcategory; - Oid outfuncoid; + FmgrInfo outflinfo; if (val_type == InvalidOid) ereport(ERROR, @@ -718,24 +716,27 @@ to_json(PG_FUNCTION_ARGS) errmsg("could not determine input data type"))); json_categorize_type(val_type, false, - &tcategory, &outfuncoid); + &tcategory, &outflinfo); - PG_RETURN_DATUM(datum_to_json(val, tcategory, outfuncoid)); + PG_RETURN_DATUM(datum_to_json(val, tcategory, &outflinfo)); } /* * Turn a Datum into JSON text. * - * tcategory and outfuncoid are from a previous call to json_categorize_type. + * tcategory is from a previous call to json_categorize_type. + * + * outflinfo is a pointer to an FmgrInfo populated by the same call to + * json_categorize_type, but it is only needed for categories where + * json_categorize_type actually populates it. */ Datum -datum_to_json(Datum val, JsonTypeCategory tcategory, Oid outfuncoid) +datum_to_json(Datum val, JsonTypeCategory tcategory, FmgrInfo *outflinfo) { StringInfoData result; initStringInfo(&result); - datum_to_json_internal(val, false, &result, tcategory, outfuncoid, - false); + datum_to_json_internal(val, false, &result, tcategory, outflinfo, false); return PointerGetDatum(cstring_to_text_with_len(result.data, result.len)); } @@ -772,16 +773,19 @@ json_agg_transfn_worker(FunctionCallInfo fcinfo, bool absent_on_null) * Make this state object in a context where it will persist for the * duration of the aggregate call. MemoryContextSwitchTo is only * needed the first time, as the StringInfo routines make sure they - * use the right context to enlarge the object if necessary. + * use the right context to enlarge the object if necessary, and + * val_flinfo will also keep track of the context used to initialize + * it. */ oldcontext = MemoryContextSwitchTo(aggcontext); state = palloc_object(JsonAggState); state->str = makeStringInfo(); + + json_categorize_type(arg_type, false, &state->val_category, + &state->val_flinfo); MemoryContextSwitchTo(oldcontext); appendStringInfoChar(state->str, '['); - json_categorize_type(arg_type, false, &state->val_category, - &state->val_output_func); } else { @@ -798,7 +802,7 @@ json_agg_transfn_worker(FunctionCallInfo fcinfo, bool absent_on_null) if (PG_ARGISNULL(1)) { datum_to_json_internal((Datum) 0, true, state->str, JSONTYPE_NULL, - InvalidOid, false); + NULL, false); PG_RETURN_POINTER(state); } @@ -813,7 +817,7 @@ json_agg_transfn_worker(FunctionCallInfo fcinfo, bool absent_on_null) } datum_to_json_internal(val, false, state->str, state->val_category, - state->val_output_func, false); + &state->val_flinfo, false); /* * The transition type for json_agg() is declared to be "internal", which @@ -991,10 +995,15 @@ json_object_agg_transfn_worker(FunctionCallInfo fcinfo, Oid arg_type; /* - * Make the StringInfo in a context where it will persist for the - * duration of the aggregate call. Switching context is only needed - * for this initial step, as the StringInfo and dynahash routines make - * sure they use the right context to enlarge the object if necessary. + * For this initial call, we need to switch to aggcontext. That's + * important for the StringInfo, for the dynahash created by the call + * to json_unique_builder_init(), and for the calls to + * json_categorize_type, which set up state->key_flinfo and + * state->val_flinfo. + * + * (It won't be necessary to switch contexts on subsequent calls to + * this function, since all of these objects are careful to make sure + * that subsequent allocations use the correct context.) */ oldcontext = MemoryContextSwitchTo(aggcontext); state = palloc_object(JsonAggState); @@ -1003,7 +1012,6 @@ json_object_agg_transfn_worker(FunctionCallInfo fcinfo, json_unique_builder_init(&state->unique_check); else memset(&state->unique_check, 0, sizeof(state->unique_check)); - MemoryContextSwitchTo(oldcontext); arg_type = get_fn_expr_argtype(fcinfo->flinfo, 1); @@ -1013,7 +1021,7 @@ json_object_agg_transfn_worker(FunctionCallInfo fcinfo, errmsg("could not determine data type for argument %d", 1))); json_categorize_type(arg_type, false, &state->key_category, - &state->key_output_func); + &state->key_flinfo); arg_type = get_fn_expr_argtype(fcinfo->flinfo, 2); @@ -1023,7 +1031,8 @@ json_object_agg_transfn_worker(FunctionCallInfo fcinfo, errmsg("could not determine data type for argument %d", 2))); json_categorize_type(arg_type, false, &state->val_category, - &state->val_output_func); + &state->val_flinfo); + MemoryContextSwitchTo(oldcontext); appendStringInfoString(state->str, "{ "); } @@ -1077,7 +1086,7 @@ json_object_agg_transfn_worker(FunctionCallInfo fcinfo, key_offset = out->len; datum_to_json_internal(arg, false, out, state->key_category, - state->key_output_func, true); + &state->key_flinfo, true); if (unique_keys) { @@ -1107,8 +1116,8 @@ json_object_agg_transfn_worker(FunctionCallInfo fcinfo, arg = PG_GETARG_DATUM(2); datum_to_json_internal(arg, PG_ARGISNULL(2), state->str, - state->val_category, - state->val_output_func, false); + state->val_category, &state->val_flinfo, + false); PG_RETURN_POINTER(state); } diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c index 864c5ac1c85..94fc3d4ecfb 100644 --- a/src/backend/utils/adt/jsonb.c +++ b/src/backend/utils/adt/jsonb.c @@ -29,9 +29,9 @@ typedef struct JsonbAggState { JsonbInState pstate; JsonTypeCategory key_category; - Oid key_output_func; + FmgrInfo key_flinfo; JsonTypeCategory val_category; - Oid val_output_func; + FmgrInfo val_flinfo; } JsonbAggState; static inline Datum jsonb_from_cstring(char *json, int len, bool unique_keys, @@ -47,11 +47,11 @@ static JsonParseErrorType jsonb_in_scalar(void *pstate, char *token, JsonTokenTy static void composite_to_jsonb(Datum composite, JsonbInState *result); static void array_dim_to_jsonb(JsonbInState *result, int dim, int ndims, int *dims, const Datum *vals, const bool *nulls, int *valcount, - JsonTypeCategory tcategory, Oid outfuncoid); + JsonTypeCategory tcategory, FmgrInfo *outflinfo); static void array_to_jsonb_internal(Datum array, JsonbInState *result); static void datum_to_jsonb_internal(Datum val, bool is_null, JsonbInState *result, - JsonTypeCategory tcategory, Oid outfuncoid, - bool key_scalar); + JsonTypeCategory tcategory, + FmgrInfo *outflinfo, bool key_scalar); static void add_jsonb(Datum val, bool is_null, JsonbInState *result, Oid val_type, bool key_scalar); static char *JsonbToCStringWorker(StringInfo out, JsonbContainer *in, int estimated_len, bool indent); @@ -617,8 +617,12 @@ add_indent(StringInfo out, bool indent, int level) /* * Turn a Datum into jsonb, adding it to the result JsonbInState. * - * tcategory and outfuncoid are from a previous call to json_categorize_type, - * except that if is_null is true then they can be invalid. + * tcategory is from a previous call to json_categorize_type, except that if + * is_null is true then it can be invalid. + * + * outflinfo is a pointer to an FmgrInfo populated by the same call to + * json_categorize_type, but it is only needed for categories where + * json_categorize_type actually populates it. * * If key_scalar is true, the value is stored as a key, so insist * it's of an acceptable type, and force it to be a jbvString. @@ -628,7 +632,7 @@ add_indent(StringInfo out, bool indent, int level) */ static void datum_to_jsonb_internal(Datum val, bool is_null, JsonbInState *result, - JsonTypeCategory tcategory, Oid outfuncoid, + JsonTypeCategory tcategory, FmgrInfo *outflinfo, bool key_scalar) { char *outputstr; @@ -691,7 +695,7 @@ datum_to_jsonb_internal(Datum val, bool is_null, JsonbInState *result, { Datum numd; - switch (outfuncoid) + switch (outflinfo->fn_oid) { case F_NUMERIC_OUT: numeric_val = DatumGetNumeric(val); @@ -725,7 +729,7 @@ datum_to_jsonb_internal(Datum val, bool is_null, JsonbInState *result, break; #endif default: - outputstr = OidOutputFunctionCall(outfuncoid, val); + outputstr = OutputFunctionCall(outflinfo, val); numd = DirectFunctionCall3(numeric_in, CStringGetDatum(outputstr), ObjectIdGetDatum(InvalidOid), @@ -739,7 +743,7 @@ datum_to_jsonb_internal(Datum val, bool is_null, JsonbInState *result, } if (numeric_to_string) { - outputstr = OidOutputFunctionCall(outfuncoid, val); + outputstr = OutputFunctionCall(outflinfo, val); jb.type = jbvString; jb.val.string.len = strlen(outputstr); jb.val.string.val = outputstr; @@ -770,7 +774,7 @@ datum_to_jsonb_internal(Datum val, bool is_null, JsonbInState *result, break; case JSONTYPE_CAST: /* cast to JSON, and then process as JSON */ - val = OidFunctionCall1(outfuncoid, val); + val = FunctionCall1(outflinfo, val); pg_fallthrough; case JSONTYPE_JSON: { @@ -828,9 +832,9 @@ datum_to_jsonb_internal(Datum val, bool is_null, JsonbInState *result, break; default: /* special-case text types to save useless palloc/memcpy ops */ - if (outfuncoid == F_TEXTOUT || - outfuncoid == F_VARCHAROUT || - outfuncoid == F_BPCHAROUT) + if (outflinfo->fn_oid == F_TEXTOUT || + outflinfo->fn_oid == F_VARCHAROUT || + outflinfo->fn_oid == F_BPCHAROUT) { text *txt = DatumGetTextPP(val); @@ -839,7 +843,7 @@ datum_to_jsonb_internal(Datum val, bool is_null, JsonbInState *result, } else { - outputstr = OidOutputFunctionCall(outfuncoid, val); + outputstr = OutputFunctionCall(outflinfo, val); jb.val.string.len = strlen(outputstr); jb.val.string.val = outputstr; } @@ -897,7 +901,7 @@ datum_to_jsonb_internal(Datum val, bool is_null, JsonbInState *result, static void array_dim_to_jsonb(JsonbInState *result, int dim, int ndims, int *dims, const Datum *vals, const bool *nulls, int *valcount, JsonTypeCategory tcategory, - Oid outfuncoid) + FmgrInfo *outflinfo) { int i; @@ -909,14 +913,14 @@ array_dim_to_jsonb(JsonbInState *result, int dim, int ndims, int *dims, const Da { if (dim + 1 == ndims) { - datum_to_jsonb_internal(vals[*valcount], nulls[*valcount], result, tcategory, - outfuncoid, false); + datum_to_jsonb_internal(vals[*valcount], nulls[*valcount], + result, tcategory, outflinfo, false); (*valcount)++; } else { array_dim_to_jsonb(result, dim + 1, ndims, dims, vals, nulls, - valcount, tcategory, outfuncoid); + valcount, tcategory, outflinfo); } } @@ -941,7 +945,7 @@ array_to_jsonb_internal(Datum array, JsonbInState *result) bool typbyval; char typalign; JsonTypeCategory tcategory; - Oid outfuncoid; + FmgrInfo outflinfo; ndim = ARR_NDIM(v); dim = ARR_DIMS(v); @@ -958,14 +962,14 @@ array_to_jsonb_internal(Datum array, JsonbInState *result) &typlen, &typbyval, &typalign); json_categorize_type(element_type, true, - &tcategory, &outfuncoid); + &tcategory, &outflinfo); deconstruct_array(v, element_type, typlen, typbyval, typalign, &elements, &nulls, &nitems); - array_dim_to_jsonb(result, 0, ndim, dim, elements, nulls, &count, tcategory, - outfuncoid); + array_dim_to_jsonb(result, 0, ndim, dim, elements, nulls, &count, + tcategory, &outflinfo); pfree(elements); pfree(nulls); @@ -1005,7 +1009,7 @@ composite_to_jsonb(Datum composite, JsonbInState *result) bool isnull; char *attname; JsonTypeCategory tcategory; - Oid outfuncoid; + FmgrInfo outflinfo; JsonbValue v; Form_pg_attribute att = TupleDescAttr(tupdesc, i); @@ -1024,15 +1028,12 @@ composite_to_jsonb(Datum composite, JsonbInState *result) val = heap_getattr(tuple, i + 1, tupdesc, &isnull); if (isnull) - { tcategory = JSONTYPE_NULL; - outfuncoid = InvalidOid; - } else json_categorize_type(att->atttypid, true, &tcategory, - &outfuncoid); + &outflinfo); - datum_to_jsonb_internal(val, isnull, result, tcategory, outfuncoid, + datum_to_jsonb_internal(val, isnull, result, tcategory, &outflinfo, false); } @@ -1053,7 +1054,7 @@ add_jsonb(Datum val, bool is_null, JsonbInState *result, Oid val_type, bool key_scalar) { JsonTypeCategory tcategory; - Oid outfuncoid; + FmgrInfo outflinfo; if (val_type == InvalidOid) ereport(ERROR, @@ -1061,15 +1062,12 @@ add_jsonb(Datum val, bool is_null, JsonbInState *result, errmsg("could not determine input data type"))); if (is_null) - { tcategory = JSONTYPE_NULL; - outfuncoid = InvalidOid; - } else json_categorize_type(val_type, true, - &tcategory, &outfuncoid); + &tcategory, &outflinfo); - datum_to_jsonb_internal(val, is_null, result, tcategory, outfuncoid, + datum_to_jsonb_internal(val, is_null, result, tcategory, &outflinfo, key_scalar); } @@ -1082,7 +1080,7 @@ to_jsonb_is_immutable(Oid typoid) { bool has_mutable = false; - json_check_mutability(typoid, true, &has_mutable); + json_check_mutability(typoid, &has_mutable); return !has_mutable; } @@ -1095,7 +1093,7 @@ to_jsonb(PG_FUNCTION_ARGS) Datum val = PG_GETARG_DATUM(0); Oid val_type = get_fn_expr_argtype(fcinfo->flinfo, 0); JsonTypeCategory tcategory; - Oid outfuncoid; + FmgrInfo outflinfo; if (val_type == InvalidOid) ereport(ERROR, @@ -1103,24 +1101,28 @@ to_jsonb(PG_FUNCTION_ARGS) errmsg("could not determine input data type"))); json_categorize_type(val_type, true, - &tcategory, &outfuncoid); + &tcategory, &outflinfo); - PG_RETURN_DATUM(datum_to_jsonb(val, tcategory, outfuncoid)); + PG_RETURN_DATUM(datum_to_jsonb(val, tcategory, &outflinfo)); } /* * Turn a Datum into jsonb. * - * tcategory and outfuncoid are from a previous call to json_categorize_type. + * tcategory is from a previous call to json_categorize_type. + * + * outflinfo is a pointer to an FmgrInfo populated by the same call to + * json_categorize_type, but it is only needed for categories where + * json_categorize_type actually populates it. */ Datum -datum_to_jsonb(Datum val, JsonTypeCategory tcategory, Oid outfuncoid) +datum_to_jsonb(Datum val, JsonTypeCategory tcategory, FmgrInfo *outflinfo) { JsonbInState result; memset(&result, 0, sizeof(JsonbInState)); - datum_to_jsonb_internal(val, false, &result, tcategory, outfuncoid, + datum_to_jsonb_internal(val, false, &result, tcategory, outflinfo, false); return JsonbPGetDatum(JsonbValueToJsonb(result.result)); @@ -1490,6 +1492,7 @@ jsonb_agg_transfn_worker(FunctionCallInfo fcinfo, bool absent_on_null) if (PG_ARGISNULL(0)) { Oid arg_type = get_fn_expr_argtype(fcinfo->flinfo, 1); + MemoryContext oldcontext; if (arg_type == InvalidOid) ereport(ERROR, @@ -1501,8 +1504,11 @@ jsonb_agg_transfn_worker(FunctionCallInfo fcinfo, bool absent_on_null) result->outcontext = aggcontext; pushJsonbValue(result, WJB_BEGIN_ARRAY, NULL); + /* make sure to set up val_flinfo in the correct context */ + oldcontext = MemoryContextSwitchTo(aggcontext); json_categorize_type(arg_type, true, &state->val_category, - &state->val_output_func); + &state->val_flinfo); + MemoryContextSwitchTo(oldcontext); } else { @@ -1522,7 +1528,7 @@ jsonb_agg_transfn_worker(FunctionCallInfo fcinfo, bool absent_on_null) val = PG_ARGISNULL(1) ? (Datum) 0 : PG_GETARG_DATUM(1); datum_to_jsonb_internal(val, PG_ARGISNULL(1), result, state->val_category, - state->val_output_func, false); + &state->val_flinfo, false); PG_RETURN_POINTER(state); } @@ -1599,6 +1605,7 @@ jsonb_object_agg_transfn_worker(FunctionCallInfo fcinfo, if (PG_ARGISNULL(0)) { Oid arg_type; + MemoryContext oldcontext; state = MemoryContextAllocZero(aggcontext, sizeof(JsonbAggState)); result = &state->pstate; @@ -1607,6 +1614,9 @@ jsonb_object_agg_transfn_worker(FunctionCallInfo fcinfo, result->parseState->unique_keys = unique_keys; result->parseState->skip_nulls = absent_on_null; + /* make sure to set up key_flinfo/val_flinfo in the correct context */ + oldcontext = MemoryContextSwitchTo(aggcontext); + arg_type = get_fn_expr_argtype(fcinfo->flinfo, 1); if (arg_type == InvalidOid) @@ -1615,7 +1625,7 @@ jsonb_object_agg_transfn_worker(FunctionCallInfo fcinfo, errmsg("could not determine input data type"))); json_categorize_type(arg_type, true, &state->key_category, - &state->key_output_func); + &state->key_flinfo); arg_type = get_fn_expr_argtype(fcinfo->flinfo, 2); @@ -1625,7 +1635,8 @@ jsonb_object_agg_transfn_worker(FunctionCallInfo fcinfo, errmsg("could not determine input data type"))); json_categorize_type(arg_type, true, &state->val_category, - &state->val_output_func); + &state->val_flinfo); + MemoryContextSwitchTo(oldcontext); } else { @@ -1656,12 +1667,12 @@ jsonb_object_agg_transfn_worker(FunctionCallInfo fcinfo, val = PG_GETARG_DATUM(1); datum_to_jsonb_internal(val, false, result, state->key_category, - state->key_output_func, true); + &state->key_flinfo, true); val = PG_ARGISNULL(2) ? (Datum) 0 : PG_GETARG_DATUM(2); datum_to_jsonb_internal(val, PG_ARGISNULL(2), result, state->val_category, - state->val_output_func, false); + &state->val_flinfo, false); PG_RETURN_POINTER(state); } diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c index 97cc3d60340..f769e90ba5e 100644 --- a/src/backend/utils/adt/jsonfuncs.c +++ b/src/backend/utils/adt/jsonfuncs.c @@ -506,6 +506,8 @@ static JsonParseErrorType transform_string_values_object_field_start(void *state static JsonParseErrorType transform_string_values_array_element_start(void *state, bool isnull); static JsonParseErrorType transform_string_values_scalar(void *state, char *token, JsonTokenType tokentype); +/* function supporting json_categorize_type/json_check_mutability */ +static Oid get_json_cast_for_type(Oid typoid); /* * pg_parse_json_or_errsave @@ -5959,25 +5961,22 @@ json_get_first_token(text *json, bool throw_error) /* * Determine how we want to print values of a given type in datum_to_json(b). * - * Given the datatype OID, return its JsonTypeCategory, as well as the type's - * output function OID. If the returned category is JSONTYPE_CAST, we return - * the OID of the type->JSON cast function instead. + * Given the datatype OID, return its JsonTypeCategory, as well as an FmgrInfo + * for the type's output function or cast function. For categories that do not + * require calling a function, outflinfo is not touched. */ void json_categorize_type(Oid typoid, bool is_jsonb, - JsonTypeCategory *tcategory, Oid *outfuncoid) + JsonTypeCategory *tcategory, FmgrInfo *outflinfo) { - bool typisvarlena; + bool use_type_output_function = false; /* Look through any domain */ typoid = getBaseType(typoid); - *outfuncoid = InvalidOid; - switch (typoid) { case BOOLOID: - *outfuncoid = F_BOOLOUT; *tcategory = JSONTYPE_BOOL; break; @@ -5987,32 +5986,29 @@ json_categorize_type(Oid typoid, bool is_jsonb, case FLOAT4OID: case FLOAT8OID: case NUMERICOID: - getTypeOutputInfo(typoid, outfuncoid, &typisvarlena); + use_type_output_function = true; *tcategory = JSONTYPE_NUMERIC; break; case DATEOID: - *outfuncoid = F_DATE_OUT; *tcategory = JSONTYPE_DATE; break; case TIMESTAMPOID: - *outfuncoid = F_TIMESTAMP_OUT; *tcategory = JSONTYPE_TIMESTAMP; break; case TIMESTAMPTZOID: - *outfuncoid = F_TIMESTAMPTZ_OUT; *tcategory = JSONTYPE_TIMESTAMPTZ; break; case JSONOID: - getTypeOutputInfo(typoid, outfuncoid, &typisvarlena); + use_type_output_function = !is_jsonb; *tcategory = JSONTYPE_JSON; break; case JSONBOID: - getTypeOutputInfo(typoid, outfuncoid, &typisvarlena); + use_type_output_function = !is_jsonb; *tcategory = is_jsonb ? JSONTYPE_JSONB : JSONTYPE_JSON; break; @@ -6020,50 +6016,35 @@ json_categorize_type(Oid typoid, bool is_jsonb, /* Check for arrays and composites */ if (OidIsValid(get_element_type(typoid)) || typoid == ANYARRAYOID || typoid == ANYCOMPATIBLEARRAYOID || typoid == RECORDARRAYOID) - { - *outfuncoid = F_ARRAY_OUT; *tcategory = JSONTYPE_ARRAY; - } else if (type_is_rowtype(typoid)) /* includes RECORDOID */ - { - *outfuncoid = F_RECORD_OUT; *tcategory = JSONTYPE_COMPOSITE; - } else { - /* - * It's probably the general case. But let's look for a cast - * to json (note: not to jsonb even if is_jsonb is true), if - * it's not built-in. - */ - *tcategory = JSONTYPE_OTHER; - if (typoid >= FirstNormalObjectId) + Oid castfunc = get_json_cast_for_type(typoid); + + if (OidIsValid(castfunc)) { - Oid castfunc; - CoercionPathType ctype; - - ctype = find_coercion_pathway(JSONOID, typoid, - COERCION_EXPLICIT, - &castfunc); - if (ctype == COERCION_PATH_FUNC && OidIsValid(castfunc)) - { - *outfuncoid = castfunc; - *tcategory = JSONTYPE_CAST; - } - else - { - /* non builtin type with no cast */ - getTypeOutputInfo(typoid, outfuncoid, &typisvarlena); - } + fmgr_info(castfunc, outflinfo); + *tcategory = JSONTYPE_CAST; } else { - /* any other builtin type */ - getTypeOutputInfo(typoid, outfuncoid, &typisvarlena); + use_type_output_function = true; + *tcategory = JSONTYPE_OTHER; } } break; } + + if (use_type_output_function) + { + Oid typoutput; + bool typisvarlena; + + getTypeOutputInfo(typoid, &typoutput, &typisvarlena); + fmgr_info(typoutput, outflinfo); + } } /* @@ -6075,11 +6056,9 @@ json_categorize_type(Oid typoid, bool is_jsonb, * If any mutable function is found, *has_mutable is set to true. */ void -json_check_mutability(Oid typoid, bool is_jsonb, bool *has_mutable) +json_check_mutability(Oid typoid, bool *has_mutable) { char att_typtype = get_typtype(typoid); - JsonTypeCategory tcategory; - Oid outfuncoid; /* since this function recurses, it could be driven to stack overflow */ check_stack_depth(); @@ -6091,7 +6070,7 @@ json_check_mutability(Oid typoid, bool is_jsonb, bool *has_mutable) if (att_typtype == TYPTYPE_DOMAIN) { - json_check_mutability(getBaseType(typoid), is_jsonb, has_mutable); + json_check_mutability(getBaseType(typoid), has_mutable); return; } else if (att_typtype == TYPTYPE_COMPOSITE) @@ -6109,7 +6088,7 @@ json_check_mutability(Oid typoid, bool is_jsonb, bool *has_mutable) if (attr->attisdropped) continue; - json_check_mutability(attr->atttypid, is_jsonb, has_mutable); + json_check_mutability(attr->atttypid, has_mutable); if (*has_mutable) break; } @@ -6118,14 +6097,12 @@ json_check_mutability(Oid typoid, bool is_jsonb, bool *has_mutable) } else if (att_typtype == TYPTYPE_RANGE) { - json_check_mutability(get_range_subtype(typoid), is_jsonb, - has_mutable); + json_check_mutability(get_range_subtype(typoid), has_mutable); return; } else if (att_typtype == TYPTYPE_MULTIRANGE) { - json_check_mutability(get_multirange_range(typoid), is_jsonb, - has_mutable); + json_check_mutability(get_multirange_range(typoid), has_mutable); return; } else @@ -6135,36 +6112,73 @@ json_check_mutability(Oid typoid, bool is_jsonb, bool *has_mutable) if (OidIsValid(att_typelem)) { /* recurse into array element type */ - json_check_mutability(att_typelem, is_jsonb, has_mutable); + json_check_mutability(att_typelem, has_mutable); return; } } - json_categorize_type(typoid, is_jsonb, &tcategory, &outfuncoid); - - switch (tcategory) + switch (typoid) { - case JSONTYPE_NULL: - case JSONTYPE_BOOL: - case JSONTYPE_NUMERIC: + case BOOLOID: + case INT2OID: + case INT4OID: + case INT8OID: + case FLOAT4OID: + case FLOAT8OID: + case NUMERICOID: + case JSONOID: + case JSONBOID: + /* known immutable */ break; - - case JSONTYPE_DATE: - case JSONTYPE_TIMESTAMP: - case JSONTYPE_TIMESTAMPTZ: + case DATEOID: + case TIMESTAMPOID: + case TIMESTAMPTZOID: *has_mutable = true; break; - - case JSONTYPE_JSON: - case JSONTYPE_JSONB: - case JSONTYPE_ARRAY: - case JSONTYPE_COMPOSITE: + case RECORDOID: + case ANYARRAYOID: + case ANYCOMPATIBLEARRAYOID: + /* XXX incorrectly treated as known immutable */ break; - case JSONTYPE_CAST: - case JSONTYPE_OTHER: - if (func_volatile(outfuncoid) != PROVOLATILE_IMMUTABLE) - *has_mutable = true; + default: + { + Oid castfunc = get_json_cast_for_type(typoid); + Oid funcoid; + + if (OidIsValid(castfunc)) + funcoid = castfunc; + else + { + bool typisvarlena; + + getTypeOutputInfo(typoid, &funcoid, &typisvarlena); + } + if (func_volatile(funcoid) != PROVOLATILE_IMMUTABLE) + *has_mutable = true; + } break; } } + +/* + * Return the OID of a cast function from typoid to JSON, or InvalidOid if + * there is no such cast. As a matter of policy, we only consider explicit, + * user-defined casts. + */ +static Oid +get_json_cast_for_type(Oid typoid) +{ + if (typoid >= FirstNormalObjectId) + { + Oid castfunc; + CoercionPathType ctype; + + ctype = find_coercion_pathway(JSONOID, typoid, + COERCION_EXPLICIT, + &castfunc); + if (ctype == COERCION_PATH_FUNC && OidIsValid(castfunc)) + return castfunc; + } + return InvalidOid; +} diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h index c61b3d624d5..8650ff57952 100644 --- a/src/include/executor/execExpr.h +++ b/src/include/executor/execExpr.h @@ -831,7 +831,7 @@ typedef struct JsonConstructorExprState struct { int category; - Oid outfuncid; + FmgrInfo outflinfo; } *arg_type_cache; /* cache for datum_to_json[b]() */ int nargs; } JsonConstructorExprState; diff --git a/src/include/utils/jsonfuncs.h b/src/include/utils/jsonfuncs.h index 27713be3aeb..96409557f29 100644 --- a/src/include/utils/jsonfuncs.h +++ b/src/include/utils/jsonfuncs.h @@ -82,13 +82,13 @@ typedef enum } JsonTypeCategory; extern void json_categorize_type(Oid typoid, bool is_jsonb, - JsonTypeCategory *tcategory, Oid *outfuncoid); -extern void json_check_mutability(Oid typoid, bool is_jsonb, - bool *has_mutable); + JsonTypeCategory *tcategory, + FmgrInfo *outflinfo); +extern void json_check_mutability(Oid typoid, bool *has_mutable); extern Datum datum_to_json(Datum val, JsonTypeCategory tcategory, - Oid outfuncoid); + FmgrInfo *outflinfo); extern Datum datum_to_jsonb(Datum val, JsonTypeCategory tcategory, - Oid outfuncoid); + FmgrInfo *outflinfo); extern Datum jsonb_from_text(text *js, bool unique_keys); extern Datum json_populate_type(Datum json_val, Oid json_type, diff --git a/src/test/regress/expected/sqljson.out b/src/test/regress/expected/sqljson.out index 0f337bda325..fedbeca7a21 100644 --- a/src/test/regress/expected/sqljson.out +++ b/src/test/regress/expected/sqljson.out @@ -1757,3 +1757,7 @@ SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': 1 RETURNING text) FORMAT JSON); (1 row) DROP FUNCTION volatile_one, stable_one; +-- JSON_ARRAY(ROW(...)) is not immutable, so it should be impossible to +-- create a generated column. +-- XXX: Currently, this is erroneously allowed. +CREATE TABLE json_array_of_row (a int, j json GENERATED ALWAYS AS (JSON_ARRAY(ROW(a))) STORED); diff --git a/src/test/regress/sql/sqljson.sql b/src/test/regress/sql/sqljson.sql index a68747733a1..77910137573 100644 --- a/src/test/regress/sql/sqljson.sql +++ b/src/test/regress/sql/sqljson.sql @@ -706,3 +706,8 @@ SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': stable_one() RETURNING text) FORMAT EXPLAIN (VERBOSE, COSTS OFF) SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': 1 RETURNING text) FORMAT JSON); SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': 1 RETURNING text) FORMAT JSON); DROP FUNCTION volatile_one, stable_one; + +-- JSON_ARRAY(ROW(...)) is not immutable, so it should be impossible to +-- create a generated column. +-- XXX: Currently, this is erroneously allowed. +CREATE TABLE json_array_of_row (a int, j json GENERATED ALWAYS AS (JSON_ARRAY(ROW(a))) STORED); -- 2.50.1 (Apple Git-155)