Restore json{b}_populate_record{set}'s ability to take type info from AS.

If the record argument is NULL and has no declared type more concrete
than RECORD, we can't extract useful information about the desired
rowtype from it.  In this case, see if we're in FROM with an AS clause,
and if so extract the needed rowtype info from AS.

It worked like this before v11, but commit 37a795a60 removed the
behavior, reasoning that it was undocumented, inefficient, and utterly
not self-consistent.  If you want to take type info from an AS clause,
you should be using the json_to_record() family of functions not the
json_populate_record() family.  Also, it was already the case that
the "populate" functions would fail for a null-valued RECORD input
(with an unfriendly "record type has not been registered" error)
when there wasn't an AS clause at hand, and it wasn't obvious that
that behavior wasn't OK when there was one.  However, it emerges
that some people were depending on this to work, and indeed the
rather off-point error message you got if you left off AS encouraged
slapping on AS without switching to the json_to_record() family.

Hence, put back the fallback behavior of looking for AS.  While at it,
improve the run-time error you get when there's no place to obtain type
info; we can do a lot better than "record type has not been registered".
(We can't, unfortunately, easily improve the parse-time error message
that leads people down this path in the first place.)

While at it, I refactored the code a bit to avoid duplicating the
same logic in several different places.

Per bug #15940 from Jaroslav Sivy.  Back-patch to v11 where the
current coding came in.  (The pre-v11 deficiencies in this area
aren't regressions, so we'll leave those branches alone.)

Patch by me, based on preliminary analysis by Dmitry Dolgov.

Discussion: https://postgr.es/m/15940-2ab76dc58ffb85b6@postgresql.org

diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 17de89d69cf57a70a2602eb4e417ea8c6c5697ab..787bc54a9ec5e713cb26e83e33343efeb384198d 100644 (file)
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -225,13 +225,13 @@ struct RecordIOData
    ColumnIOData columns[FLEXIBLE_ARRAY_MEMBER];
 };
 
-/* per-query cache for populate_recordset */
-typedef struct PopulateRecordsetCache
+/* per-query cache for populate_record_worker and populate_recordset_worker */
+typedef struct PopulateRecordCache
 {
    Oid         argtype;        /* declared type of the record argument */
    ColumnIOData c;             /* metadata cache for populate_composite() */
    MemoryContext fn_mcxt;      /* where this is stored */
-} PopulateRecordsetCache;
+} PopulateRecordCache;
 
 /* per-call state for populate_recordset */
 typedef struct PopulateRecordsetState
@@ -244,16 +244,9 @@ typedef struct PopulateRecordsetState
    JsonTokenType saved_token_type;
    Tuplestorestate *tuple_store;
    HeapTupleHeader rec;
-   PopulateRecordsetCache *cache;
+   PopulateRecordCache *cache;
 } PopulateRecordsetState;
 
-/* structure to cache metadata needed for populate_record_worker() */
-typedef struct PopulateRecordCache
-{
-   Oid         argtype;        /* declared type of the record argument */
-   ColumnIOData c;             /* metadata cache for populate_composite() */
-} PopulateRecordCache;
-
 /* common data for populate_array_json() and populate_array_dim_jsonb() */
 typedef struct PopulateArrayContext
 {
@@ -429,6 +422,12 @@ static Datum populate_record_worker(FunctionCallInfo fcinfo, const char *funcnam
 static HeapTupleHeader populate_record(TupleDesc tupdesc, RecordIOData **record_p,
                HeapTupleHeader defaultval, MemoryContext mcxt,
                JsObject *obj);
+static void get_record_type_from_argument(FunctionCallInfo fcinfo,
+                                         const char *funcname,
+                                         PopulateRecordCache *cache);
+static void get_record_type_from_query(FunctionCallInfo fcinfo,
+                                      const char *funcname,
+                                      PopulateRecordCache *cache);
 static void JsValueToJsObject(JsValue *jsv, JsObject *jso);
 static Datum populate_composite(CompositeIOData *io, Oid typid,
                   const char *colname, MemoryContext mcxt,
@@ -3202,6 +3201,70 @@ populate_record(TupleDesc tupdesc,
    return res->t_data;
 }
 
+/*
+ * Setup for json{b}_populate_record{set}: result type will be same as first
+ * argument's type --- unless first argument is "null::record", which we can't
+ * extract type info from; we handle that later.
+ */
+static void
+get_record_type_from_argument(FunctionCallInfo fcinfo,
+                             const char *funcname,
+                             PopulateRecordCache *cache)
+{
+   cache->argtype = get_fn_expr_argtype(fcinfo->flinfo, 0);
+   prepare_column_cache(&cache->c,
+                        cache->argtype, -1,
+                        cache->fn_mcxt, false);
+   if (cache->c.typcat != TYPECAT_COMPOSITE &&
+       cache->c.typcat != TYPECAT_COMPOSITE_DOMAIN)
+       ereport(ERROR,
+               (errcode(ERRCODE_DATATYPE_MISMATCH),
+       /* translator: %s is a function name, eg json_to_record */
+                errmsg("first argument of %s must be a row type",
+                       funcname)));
+}
+
+/*
+ * Setup for json{b}_to_record{set}: result type is specified by calling
+ * query.  We'll also use this code for json{b}_populate_record{set},
+ * if we discover that the first argument is a null of type RECORD.
+ *
+ * Here it is syntactically impossible to specify the target type
+ * as domain-over-composite.
+ */
+static void
+get_record_type_from_query(FunctionCallInfo fcinfo,
+                          const char *funcname,
+                          PopulateRecordCache *cache)
+{
+   TupleDesc   tupdesc;
+   MemoryContext old_cxt;
+
+   if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+       ereport(ERROR,
+               (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+       /* translator: %s is a function name, eg json_to_record */
+                errmsg("could not determine row type for result of %s",
+                       funcname),
+                errhint("Provide a non-null record argument, "
+                        "or call the function in the FROM clause "
+                        "using a column definition list.")));
+
+   Assert(tupdesc);
+   cache->argtype = tupdesc->tdtypeid;
+
+   /* If we go through this more than once, avoid memory leak */
+   if (cache->c.io.composite.tupdesc)
+       FreeTupleDesc(cache->c.io.composite.tupdesc);
+
+   /* Save identified tupdesc */
+   old_cxt = MemoryContextSwitchTo(cache->fn_mcxt);
+   cache->c.io.composite.tupdesc = CreateTupleDescCopy(tupdesc);
+   cache->c.io.composite.base_typid = tupdesc->tdtypeid;
+   cache->c.io.composite.base_typmod = tupdesc->tdtypmod;
+   MemoryContextSwitchTo(old_cxt);
+}
+
 /*
  * common worker for json{b}_populate_record() and json{b}_to_record()
  * is_json and have_record_arg identify the specific function
@@ -3227,63 +3290,24 @@ populate_record_worker(FunctionCallInfo fcinfo, const char *funcname,
    {
        fcinfo->flinfo->fn_extra = cache =
            MemoryContextAllocZero(fnmcxt, sizeof(*cache));
+       cache->fn_mcxt = fnmcxt;
 
        if (have_record_arg)
-       {
-           /*
-            * json{b}_populate_record case: result type will be same as first
-            * argument's.
-            */
-           cache->argtype = get_fn_expr_argtype(fcinfo->flinfo, 0);
-           prepare_column_cache(&cache->c,
-                                cache->argtype, -1,
-                                fnmcxt, false);
-           if (cache->c.typcat != TYPECAT_COMPOSITE &&
-               cache->c.typcat != TYPECAT_COMPOSITE_DOMAIN)
-               ereport(ERROR,
-                       (errcode(ERRCODE_DATATYPE_MISMATCH),
-                        errmsg("first argument of %s must be a row type",
-                               funcname)));
-       }
+           get_record_type_from_argument(fcinfo, funcname, cache);
        else
-       {
-           /*
-            * json{b}_to_record case: result type is specified by calling
-            * query.  Here it is syntactically impossible to specify the
-            * target type as domain-over-composite.
-            */
-           TupleDesc   tupdesc;
-           MemoryContext old_cxt;
-
-           if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
-               ereport(ERROR,
-                       (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                        errmsg("function returning record called in context "
-                               "that cannot accept type record"),
-                        errhint("Try calling the function in the FROM clause "
-                                "using a column definition list.")));
-
-           Assert(tupdesc);
-           cache->argtype = tupdesc->tdtypeid;
-
-           /* Save identified tupdesc */
-           old_cxt = MemoryContextSwitchTo(fnmcxt);
-           cache->c.io.composite.tupdesc = CreateTupleDescCopy(tupdesc);
-           cache->c.io.composite.base_typid = tupdesc->tdtypeid;
-           cache->c.io.composite.base_typmod = tupdesc->tdtypmod;
-           MemoryContextSwitchTo(old_cxt);
-       }
+           get_record_type_from_query(fcinfo, funcname, cache);
    }
 
    /* Collect record arg if we have one */
-   if (have_record_arg && !PG_ARGISNULL(0))
+   if (!have_record_arg)
+       rec = NULL;             /* it's json{b}_to_record() */
+   else if (!PG_ARGISNULL(0))
    {
        rec = PG_GETARG_HEAPTUPLEHEADER(0);
 
        /*
         * When declared arg type is RECORD, identify actual record type from
-        * the tuple itself.  Note the lookup_rowtype_tupdesc call in
-        * update_cached_tupdesc will fail if we're unable to do this.
+        * the tuple itself.
         */
        if (cache->argtype == RECORDOID)
        {
@@ -3292,8 +3316,21 @@ populate_record_worker(FunctionCallInfo fcinfo, const char *funcname,
        }
    }
    else
+   {
        rec = NULL;
 
+       /*
+        * When declared arg type is RECORD, identify actual record type from
+        * calling query, or fail if we can't.
+        */
+       if (cache->argtype == RECORDOID)
+       {
+           get_record_type_from_query(fcinfo, funcname, cache);
+           /* This can't change argtype, which is important for next time */
+           Assert(cache->argtype == RECORDOID);
+       }
+   }
+
    /* If no JSON argument, just return the record (if any) unchanged */
    if (PG_ARGISNULL(json_arg_num))
    {
@@ -3517,7 +3554,7 @@ json_to_recordset(PG_FUNCTION_ARGS)
 static void
 populate_recordset_record(PopulateRecordsetState *state, JsObject *obj)
 {
-   PopulateRecordsetCache *cache = state->cache;
+   PopulateRecordCache *cache = state->cache;
    HeapTupleHeader tuphead;
    HeapTupleData tuple;
 
@@ -3559,7 +3596,7 @@ populate_recordset_worker(FunctionCallInfo fcinfo, const char *funcname,
    ReturnSetInfo *rsi;
    MemoryContext old_cxt;
    HeapTupleHeader rec;
-   PopulateRecordsetCache *cache = fcinfo->flinfo->fn_extra;
+   PopulateRecordCache *cache = fcinfo->flinfo->fn_extra;
    PopulateRecordsetState *state;
 
    rsi = (ReturnSetInfo *) fcinfo->resultinfo;
@@ -3585,60 +3622,21 @@ populate_recordset_worker(FunctionCallInfo fcinfo, const char *funcname,
        cache->fn_mcxt = fcinfo->flinfo->fn_mcxt;
 
        if (have_record_arg)
-       {
-           /*
-            * json{b}_populate_recordset case: result type will be same as
-            * first argument's.
-            */
-           cache->argtype = get_fn_expr_argtype(fcinfo->flinfo, 0);
-           prepare_column_cache(&cache->c,
-                                cache->argtype, -1,
-                                cache->fn_mcxt, false);
-           if (cache->c.typcat != TYPECAT_COMPOSITE &&
-               cache->c.typcat != TYPECAT_COMPOSITE_DOMAIN)
-               ereport(ERROR,
-                       (errcode(ERRCODE_DATATYPE_MISMATCH),
-                        errmsg("first argument of %s must be a row type",
-                               funcname)));
-       }
+           get_record_type_from_argument(fcinfo, funcname, cache);
        else
-       {
-           /*
-            * json{b}_to_recordset case: result type is specified by calling
-            * query.  Here it is syntactically impossible to specify the
-            * target type as domain-over-composite.
-            */
-           TupleDesc   tupdesc;
-
-           if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
-               ereport(ERROR,
-                       (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                        errmsg("function returning record called in context "
-                               "that cannot accept type record"),
-                        errhint("Try calling the function in the FROM clause "
-                                "using a column definition list.")));
-
-           Assert(tupdesc);
-           cache->argtype = tupdesc->tdtypeid;
-
-           /* Save identified tupdesc */
-           old_cxt = MemoryContextSwitchTo(cache->fn_mcxt);
-           cache->c.io.composite.tupdesc = CreateTupleDescCopy(tupdesc);
-           cache->c.io.composite.base_typid = tupdesc->tdtypeid;
-           cache->c.io.composite.base_typmod = tupdesc->tdtypmod;
-           MemoryContextSwitchTo(old_cxt);
-       }
+           get_record_type_from_query(fcinfo, funcname, cache);
    }
 
    /* Collect record arg if we have one */
-   if (have_record_arg && !PG_ARGISNULL(0))
+   if (!have_record_arg)
+       rec = NULL;             /* it's json{b}_to_recordset() */
+   else if (!PG_ARGISNULL(0))
    {
        rec = PG_GETARG_HEAPTUPLEHEADER(0);
 
        /*
         * When declared arg type is RECORD, identify actual record type from
-        * the tuple itself.  Note the lookup_rowtype_tupdesc call in
-        * update_cached_tupdesc will fail if we're unable to do this.
+        * the tuple itself.
         */
        if (cache->argtype == RECORDOID)
        {
@@ -3647,8 +3645,21 @@ populate_recordset_worker(FunctionCallInfo fcinfo, const char *funcname,
        }
    }
    else
+   {
        rec = NULL;
 
+       /*
+        * When declared arg type is RECORD, identify actual record type from
+        * calling query, or fail if we can't.
+        */
+       if (cache->argtype == RECORDOID)
+       {
+           get_record_type_from_query(fcinfo, funcname, cache);
+           /* This can't change argtype, which is important for next time */
+           Assert(cache->argtype == RECORDOID);
+       }
+   }
+
    /* if the json is null send back an empty set */
    if (PG_ARGISNULL(json_arg_num))
        PG_RETURN_NULL();

diff --git a/src/test/regress/expected/json.out b/src/test/regress/expected/json.out
index a68bf72b312e873b66eb7fcbc21ff1ff0a2deecb..e9c44d3546b0693e955d2d772dd1797d870d4e96 100644 (file)
--- a/src/test/regress/expected/json.out
+++ b/src/test/regress/expected/json.out
@@ -1744,13 +1744,21 @@ SELECT rec FROM json_populate_record(
 
 -- anonymous record type
 SELECT json_populate_record(null::record, '{"x": 0, "y": 1}');
-ERROR:  record type has not been registered
+ERROR:  could not determine row type for result of json_populate_record
+HINT:  Provide a non-null record argument, or call the function in the FROM clause using a column definition list.
 SELECT json_populate_record(row(1,2), '{"f1": 0, "f2": 1}');
  json_populate_record 
 ----------------------
  (0,1)
 (1 row)
 
+SELECT * FROM
+  json_populate_record(null::record, '{"x": 776}') AS (x int, y int);
+  x  | y 
+-----+---
+ 776 |  
+(1 row)
+
 -- composite domain
 SELECT json_populate_record(null::j_ordered_pair, '{"x": 0, "y": 1}');
  json_populate_record 
@@ -1834,7 +1842,8 @@ select * from json_populate_recordset(row('def',99,null)::jpop,'[{"a":[100,200,3
 
 -- anonymous record type
 SELECT json_populate_recordset(null::record, '[{"x": 0, "y": 1}]');
-ERROR:  record type has not been registered
+ERROR:  could not determine row type for result of json_populate_recordset
+HINT:  Provide a non-null record argument, or call the function in the FROM clause using a column definition list.
 SELECT json_populate_recordset(row(1,2), '[{"f1": 0, "f2": 1}]');
  json_populate_recordset 
 -------------------------
@@ -1851,9 +1860,17 @@ FROM (VALUES (1),(2)) v(i);
  2 | (2,43)
 (4 rows)
 
+SELECT * FROM
+  json_populate_recordset(null::record, '[{"x": 776}]') AS (x int, y int);
+  x  | y 
+-----+---
+ 776 |  
+(1 row)
+
 -- empty array is a corner case
 SELECT json_populate_recordset(null::record, '[]');
-ERROR:  record type has not been registered
+ERROR:  could not determine row type for result of json_populate_recordset
+HINT:  Provide a non-null record argument, or call the function in the FROM clause using a column definition list.
 SELECT json_populate_recordset(row(1,2), '[]');
  json_populate_recordset 
 -------------------------
@@ -1864,6 +1881,12 @@ SELECT * FROM json_populate_recordset(NULL::jpop,'[]') q;
 ---+---+---
 (0 rows)
 
+SELECT * FROM
+  json_populate_recordset(null::record, '[]') AS (x int, y int);
+ x | y 
+---+---
+(0 rows)
+
 -- composite domain
 SELECT json_populate_recordset(null::j_ordered_pair, '[{"x": 0, "y": 1}]');
  json_populate_recordset 

diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index 3dfd626c410b3dba8c0f4bca635be24205d48989..bd6fc2e47ddf067591dc56ab99be29191c9d70d4 100644 (file)
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -2433,13 +2433,21 @@ SELECT rec FROM jsonb_populate_record(
 
 -- anonymous record type
 SELECT jsonb_populate_record(null::record, '{"x": 0, "y": 1}');
-ERROR:  record type has not been registered
+ERROR:  could not determine row type for result of jsonb_populate_record
+HINT:  Provide a non-null record argument, or call the function in the FROM clause using a column definition list.
 SELECT jsonb_populate_record(row(1,2), '{"f1": 0, "f2": 1}');
  jsonb_populate_record 
 -----------------------
  (0,1)
 (1 row)
 
+SELECT * FROM
+  jsonb_populate_record(null::record, '{"x": 776}') AS (x int, y int);
+  x  | y 
+-----+---
+ 776 |  
+(1 row)
+
 -- composite domain
 SELECT jsonb_populate_record(null::jb_ordered_pair, '{"x": 0, "y": 1}');
  jsonb_populate_record 
@@ -2516,7 +2524,8 @@ SELECT * FROM jsonb_populate_recordset(row('def',99,NULL)::jbpop,'[{"a":[100,200
 
 -- anonymous record type
 SELECT jsonb_populate_recordset(null::record, '[{"x": 0, "y": 1}]');
-ERROR:  record type has not been registered
+ERROR:  could not determine row type for result of jsonb_populate_recordset
+HINT:  Provide a non-null record argument, or call the function in the FROM clause using a column definition list.
 SELECT jsonb_populate_recordset(row(1,2), '[{"f1": 0, "f2": 1}]');
  jsonb_populate_recordset 
 --------------------------
@@ -2533,9 +2542,17 @@ FROM (VALUES (1),(2)) v(i);
  2 | (2,43)
 (4 rows)
 
+SELECT * FROM
+  jsonb_populate_recordset(null::record, '[{"x": 776}]') AS (x int, y int);
+  x  | y 
+-----+---
+ 776 |  
+(1 row)
+
 -- empty array is a corner case
 SELECT jsonb_populate_recordset(null::record, '[]');
-ERROR:  record type has not been registered
+ERROR:  could not determine row type for result of jsonb_populate_recordset
+HINT:  Provide a non-null record argument, or call the function in the FROM clause using a column definition list.
 SELECT jsonb_populate_recordset(row(1,2), '[]');
  jsonb_populate_recordset 
 --------------------------
@@ -2546,6 +2563,12 @@ SELECT * FROM jsonb_populate_recordset(NULL::jbpop,'[]') q;
 ---+---+---
 (0 rows)
 
+SELECT * FROM
+  jsonb_populate_recordset(null::record, '[]') AS (x int, y int);
+ x | y 
+---+---
+(0 rows)
+
 -- composite domain
 SELECT jsonb_populate_recordset(null::jb_ordered_pair, '[{"x": 0, "y": 1}]');
  jsonb_populate_recordset 

diff --git a/src/test/regress/sql/json.sql b/src/test/regress/sql/json.sql
index 184c5029e91a01390a14b123f492e493bcb4eb25..44fd7e406674f46362ee47dde2d31a6170d52e72 100644 (file)
--- a/src/test/regress/sql/json.sql
+++ b/src/test/regress/sql/json.sql
@@ -522,6 +522,8 @@ SELECT rec FROM json_populate_record(
 -- anonymous record type
 SELECT json_populate_record(null::record, '{"x": 0, "y": 1}');
 SELECT json_populate_record(row(1,2), '{"f1": 0, "f2": 1}');
+SELECT * FROM
+  json_populate_record(null::record, '{"x": 776}') AS (x int, y int);
 
 -- composite domain
 SELECT json_populate_record(null::j_ordered_pair, '{"x": 0, "y": 1}');
@@ -549,11 +551,15 @@ SELECT json_populate_recordset(null::record, '[{"x": 0, "y": 1}]');
 SELECT json_populate_recordset(row(1,2), '[{"f1": 0, "f2": 1}]');
 SELECT i, json_populate_recordset(row(i,50), '[{"f1":"42"},{"f2":"43"}]')
 FROM (VALUES (1),(2)) v(i);
+SELECT * FROM
+  json_populate_recordset(null::record, '[{"x": 776}]') AS (x int, y int);
 
 -- empty array is a corner case
 SELECT json_populate_recordset(null::record, '[]');
 SELECT json_populate_recordset(row(1,2), '[]');
 SELECT * FROM json_populate_recordset(NULL::jpop,'[]') q;
+SELECT * FROM
+  json_populate_recordset(null::record, '[]') AS (x int, y int);
 
 -- composite domain
 SELECT json_populate_recordset(null::j_ordered_pair, '[{"x": 0, "y": 1}]');

diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql
index 227f1fcd27ee4b86395abfccd632707bdea40478..a1831adf33dd0ef753bdc87782f5fc4d123c1102 100644 (file)
--- a/src/test/regress/sql/jsonb.sql
+++ b/src/test/regress/sql/jsonb.sql
@@ -642,6 +642,8 @@ SELECT rec FROM jsonb_populate_record(
 -- anonymous record type
 SELECT jsonb_populate_record(null::record, '{"x": 0, "y": 1}');
 SELECT jsonb_populate_record(row(1,2), '{"f1": 0, "f2": 1}');
+SELECT * FROM
+  jsonb_populate_record(null::record, '{"x": 776}') AS (x int, y int);
 
 -- composite domain
 SELECT jsonb_populate_record(null::jb_ordered_pair, '{"x": 0, "y": 1}');
@@ -665,11 +667,15 @@ SELECT jsonb_populate_recordset(null::record, '[{"x": 0, "y": 1}]');
 SELECT jsonb_populate_recordset(row(1,2), '[{"f1": 0, "f2": 1}]');
 SELECT i, jsonb_populate_recordset(row(i,50), '[{"f1":"42"},{"f2":"43"}]')
 FROM (VALUES (1),(2)) v(i);
+SELECT * FROM
+  jsonb_populate_recordset(null::record, '[{"x": 776}]') AS (x int, y int);
 
 -- empty array is a corner case
 SELECT jsonb_populate_recordset(null::record, '[]');
 SELECT jsonb_populate_recordset(row(1,2), '[]');
 SELECT * FROM jsonb_populate_recordset(NULL::jbpop,'[]') q;
+SELECT * FROM
+  jsonb_populate_recordset(null::record, '[]') AS (x int, y int);
 
 -- composite domain
 SELECT jsonb_populate_recordset(null::jb_ordered_pair, '[{"x": 0, "y": 1}]');