Fix crash in json{b}_populate_recordset() and json{b}_to_recordset().

As of commit 37a795a60, populate_recordset_worker() tried to pass back
(as rsi.setDesc) a tupdesc that it also had cached in its fn_extra.
But the core executor would free the passed-back tupdesc, risking a
crash if the function were called again in the same query.  The safest
and least invasive way to fix that is to make an extra tupdesc copy
to pass back.

While at it, I failed to resist the temptation to get rid of unnecessary
get_fn_expr_argtype() calls here and in populate_record_worker().

Per report from Dmitry Dolgov; thanks to Michael Paquier and
Andrew Gierth for investigation and discussion.

Discussion: https://postgr.es/m/CA+q6zcWzN9ztCfR47ZwgTr1KLnuO6BAY6FurxXhovP4hxr+yOQ@mail.gmail.com

1 files changed, 35 insertions, 23 deletions

diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index e358b5ad133..06f8d281685 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -421,9 +421,9 @@ static void sn_scalar(void *state, char *token, JsonTokenType tokentype);
 
 /* worker functions for populate_record, to_record, populate_recordset and to_recordset */
 static Datum populate_recordset_worker(FunctionCallInfo fcinfo, const char *funcname,
-						  bool have_record_arg);
+						  bool is_json, bool have_record_arg);
 static Datum populate_record_worker(FunctionCallInfo fcinfo, const char *funcname,
-					   bool have_record_arg);
+					   bool is_json, bool have_record_arg);
 
 /* helper functions for populate_record[set] */
 static HeapTupleHeader populate_record(TupleDesc tupdesc, RecordIOData **record_p,
@@ -2296,25 +2296,29 @@ elements_scalar(void *state, char *token, JsonTokenType tokentype)
 Datum
 jsonb_populate_record(PG_FUNCTION_ARGS)
 {
-	return populate_record_worker(fcinfo, "jsonb_populate_record", true);
+	return populate_record_worker(fcinfo, "jsonb_populate_record",
+								  false, true);
 }
 
 Datum
 jsonb_to_record(PG_FUNCTION_ARGS)
 {
-	return populate_record_worker(fcinfo, "jsonb_to_record", false);
+	return populate_record_worker(fcinfo, "jsonb_to_record",
+								  false, false);
 }
 
 Datum
 json_populate_record(PG_FUNCTION_ARGS)
 {
-	return populate_record_worker(fcinfo, "json_populate_record", true);
+	return populate_record_worker(fcinfo, "json_populate_record",
+								  true, true);
 }
 
 Datum
 json_to_record(PG_FUNCTION_ARGS)
 {
-	return populate_record_worker(fcinfo, "json_to_record", false);
+	return populate_record_worker(fcinfo, "json_to_record",
+								  true, false);
 }
 
 /* helper function for diagnostics */
@@ -3203,12 +3207,15 @@ populate_record(TupleDesc tupdesc,
 	return res->t_data;
 }
 
+/*
+ * common worker for json{b}_populate_record() and json{b}_to_record()
+ * is_json and have_record_arg identify the specific function
+ */
 static Datum
 populate_record_worker(FunctionCallInfo fcinfo, const char *funcname,
-					   bool have_record_arg)
+					   bool is_json, bool have_record_arg)
 {
 	int			json_arg_num = have_record_arg ? 1 : 0;
-	Oid			jtype = get_fn_expr_argtype(fcinfo->flinfo, json_arg_num);
 	JsValue		jsv = {0};
 	HeapTupleHeader rec;
 	Datum		rettuple;
@@ -3216,8 +3223,6 @@ populate_record_worker(FunctionCallInfo fcinfo, const char *funcname,
 	MemoryContext fnmcxt = fcinfo->flinfo->fn_mcxt;
 	PopulateRecordCache *cache = fcinfo->flinfo->fn_extra;
 
-	Assert(jtype == JSONOID || jtype == JSONBOID);
-
 	/*
 	 * If first time through, identify input/result record type.  Note that
 	 * this stanza looks only at fcinfo context, which can't change during the
@@ -3303,9 +3308,9 @@ populate_record_worker(FunctionCallInfo fcinfo, const char *funcname,
 			PG_RETURN_NULL();
 	}
 
-	jsv.is_json = jtype == JSONOID;
+	jsv.is_json = is_json;
 
-	if (jsv.is_json)
+	if (is_json)
 	{
 		text	   *json = PG_GETARG_TEXT_PP(json_arg_num);
 
@@ -3489,25 +3494,29 @@ hash_scalar(void *state, char *token, JsonTokenType tokentype)
 Datum
 jsonb_populate_recordset(PG_FUNCTION_ARGS)
 {
-	return populate_recordset_worker(fcinfo, "jsonb_populate_recordset", true);
+	return populate_recordset_worker(fcinfo, "jsonb_populate_recordset",
+									 false, true);
 }
 
 Datum
 jsonb_to_recordset(PG_FUNCTION_ARGS)
 {
-	return populate_recordset_worker(fcinfo, "jsonb_to_recordset", false);
+	return populate_recordset_worker(fcinfo, "jsonb_to_recordset",
+									 false, false);
 }
 
 Datum
 json_populate_recordset(PG_FUNCTION_ARGS)
 {
-	return populate_recordset_worker(fcinfo, "json_populate_recordset", true);
+	return populate_recordset_worker(fcinfo, "json_populate_recordset",
+									 true, true);
 }
 
 Datum
 json_to_recordset(PG_FUNCTION_ARGS)
 {
-	return populate_recordset_worker(fcinfo, "json_to_recordset", false);
+	return populate_recordset_worker(fcinfo, "json_to_recordset",
+									 true, false);
 }
 
 static void
@@ -3544,14 +3553,14 @@ populate_recordset_record(PopulateRecordsetState *state, JsObject *obj)
 }
 
 /*
- * common worker for json_populate_recordset() and json_to_recordset()
+ * common worker for json{b}_populate_recordset() and json{b}_to_recordset()
+ * is_json and have_record_arg identify the specific function
  */
 static Datum
 populate_recordset_worker(FunctionCallInfo fcinfo, const char *funcname,
-						  bool have_record_arg)
+						  bool is_json, bool have_record_arg)
 {
 	int			json_arg_num = have_record_arg ? 1 : 0;
-	Oid			jtype = get_fn_expr_argtype(fcinfo->flinfo, json_arg_num);
 	ReturnSetInfo *rsi;
 	MemoryContext old_cxt;
 	HeapTupleHeader rec;
@@ -3662,7 +3671,7 @@ populate_recordset_worker(FunctionCallInfo fcinfo, const char *funcname,
 	state->cache = cache;
 	state->rec = rec;
 
-	if (jtype == JSONOID)
+	if (is_json)
 	{
 		text	   *json = PG_GETARG_TEXT_PP(json_arg_num);
 		JsonLexContext *lex;
@@ -3693,8 +3702,6 @@ populate_recordset_worker(FunctionCallInfo fcinfo, const char *funcname,
 		bool		skipNested = false;
 		JsonbIteratorToken r;
 
-		Assert(jtype == JSONBOID);
-
 		if (JB_ROOT_IS_SCALAR(jb) || !JB_ROOT_IS_ARRAY(jb))
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
@@ -3726,8 +3733,13 @@ populate_recordset_worker(FunctionCallInfo fcinfo, const char *funcname,
 		}
 	}
 
+	/*
+	 * Note: we must copy the cached tupdesc because the executor will free
+	 * the passed-back setDesc, but we want to hang onto the cache in case
+	 * we're called again in the same query.
+	 */
 	rsi->setResult = state->tuple_store;
-	rsi->setDesc = cache->c.io.composite.tupdesc;
+	rsi->setDesc = CreateTupleDescCopy(cache->c.io.composite.tupdesc);
 
 	PG_RETURN_NULL();
 }