Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit e136a0d

Browse files
committed
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 37a795a 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
1 parent 4c01a11 commit e136a0d

File tree

5 files changed

+179
-110
lines changed

5 files changed

+179
-110
lines changed

src/backend/utils/adt/jsonfuncs.c

Lines changed: 115 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -225,13 +225,13 @@ struct RecordIOData
225225
ColumnIOData columns[FLEXIBLE_ARRAY_MEMBER];
226226
};
227227

228-
/* per-query cache for populate_recordset */
229-
typedef struct PopulateRecordsetCache
228+
/* per-query cache for populate_record_worker and populate_recordset_worker */
229+
typedef struct PopulateRecordCache
230230
{
231231
Oid argtype; /* declared type of the record argument */
232232
ColumnIOData c; /* metadata cache for populate_composite() */
233233
MemoryContext fn_mcxt; /* where this is stored */
234-
} PopulateRecordsetCache;
234+
} PopulateRecordCache;
235235

236236
/* per-call state for populate_recordset */
237237
typedef struct PopulateRecordsetState
@@ -244,16 +244,9 @@ typedef struct PopulateRecordsetState
244244
JsonTokenType saved_token_type;
245245
Tuplestorestate *tuple_store;
246246
HeapTupleHeader rec;
247-
PopulateRecordsetCache *cache;
247+
PopulateRecordCache *cache;
248248
} PopulateRecordsetState;
249249

250-
/* structure to cache metadata needed for populate_record_worker() */
251-
typedef struct PopulateRecordCache
252-
{
253-
Oid argtype; /* declared type of the record argument */
254-
ColumnIOData c; /* metadata cache for populate_composite() */
255-
} PopulateRecordCache;
256-
257250
/* common data for populate_array_json() and populate_array_dim_jsonb() */
258251
typedef struct PopulateArrayContext
259252
{
@@ -429,6 +422,12 @@ static Datum populate_record_worker(FunctionCallInfo fcinfo, const char *funcnam
429422
static HeapTupleHeader populate_record(TupleDesc tupdesc, RecordIOData **record_p,
430423
HeapTupleHeader defaultval, MemoryContext mcxt,
431424
JsObject *obj);
425+
static void get_record_type_from_argument(FunctionCallInfo fcinfo,
426+
const char *funcname,
427+
PopulateRecordCache *cache);
428+
static void get_record_type_from_query(FunctionCallInfo fcinfo,
429+
const char *funcname,
430+
PopulateRecordCache *cache);
432431
static void JsValueToJsObject(JsValue *jsv, JsObject *jso);
433432
static Datum populate_composite(CompositeIOData *io, Oid typid,
434433
const char *colname, MemoryContext mcxt,
@@ -3202,6 +3201,70 @@ populate_record(TupleDesc tupdesc,
32023201
return res->t_data;
32033202
}
32043203

3204+
/*
3205+
* Setup for json{b}_populate_record{set}: result type will be same as first
3206+
* argument's type --- unless first argument is "null::record", which we can't
3207+
* extract type info from; we handle that later.
3208+
*/
3209+
static void
3210+
get_record_type_from_argument(FunctionCallInfo fcinfo,
3211+
const char *funcname,
3212+
PopulateRecordCache *cache)
3213+
{
3214+
cache->argtype = get_fn_expr_argtype(fcinfo->flinfo, 0);
3215+
prepare_column_cache(&cache->c,
3216+
cache->argtype, -1,
3217+
cache->fn_mcxt, false);
3218+
if (cache->c.typcat != TYPECAT_COMPOSITE &&
3219+
cache->c.typcat != TYPECAT_COMPOSITE_DOMAIN)
3220+
ereport(ERROR,
3221+
(errcode(ERRCODE_DATATYPE_MISMATCH),
3222+
/* translator: %s is a function name, eg json_to_record */
3223+
errmsg("first argument of %s must be a row type",
3224+
funcname)));
3225+
}
3226+
3227+
/*
3228+
* Setup for json{b}_to_record{set}: result type is specified by calling
3229+
* query. We'll also use this code for json{b}_populate_record{set},
3230+
* if we discover that the first argument is a null of type RECORD.
3231+
*
3232+
* Here it is syntactically impossible to specify the target type
3233+
* as domain-over-composite.
3234+
*/
3235+
static void
3236+
get_record_type_from_query(FunctionCallInfo fcinfo,
3237+
const char *funcname,
3238+
PopulateRecordCache *cache)
3239+
{
3240+
TupleDesc tupdesc;
3241+
MemoryContext old_cxt;
3242+
3243+
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
3244+
ereport(ERROR,
3245+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
3246+
/* translator: %s is a function name, eg json_to_record */
3247+
errmsg("could not determine row type for result of %s",
3248+
funcname),
3249+
errhint("Provide a non-null record argument, "
3250+
"or call the function in the FROM clause "
3251+
"using a column definition list.")));
3252+
3253+
Assert(tupdesc);
3254+
cache->argtype = tupdesc->tdtypeid;
3255+
3256+
/* If we go through this more than once, avoid memory leak */
3257+
if (cache->c.io.composite.tupdesc)
3258+
FreeTupleDesc(cache->c.io.composite.tupdesc);
3259+
3260+
/* Save identified tupdesc */
3261+
old_cxt = MemoryContextSwitchTo(cache->fn_mcxt);
3262+
cache->c.io.composite.tupdesc = CreateTupleDescCopy(tupdesc);
3263+
cache->c.io.composite.base_typid = tupdesc->tdtypeid;
3264+
cache->c.io.composite.base_typmod = tupdesc->tdtypmod;
3265+
MemoryContextSwitchTo(old_cxt);
3266+
}
3267+
32053268
/*
32063269
* common worker for json{b}_populate_record() and json{b}_to_record()
32073270
* is_json and have_record_arg identify the specific function
@@ -3227,63 +3290,24 @@ populate_record_worker(FunctionCallInfo fcinfo, const char *funcname,
32273290
{
32283291
fcinfo->flinfo->fn_extra = cache =
32293292
MemoryContextAllocZero(fnmcxt, sizeof(*cache));
3293+
cache->fn_mcxt = fnmcxt;
32303294

32313295
if (have_record_arg)
3232-
{
3233-
/*
3234-
* json{b}_populate_record case: result type will be same as first
3235-
* argument's.
3236-
*/
3237-
cache->argtype = get_fn_expr_argtype(fcinfo->flinfo, 0);
3238-
prepare_column_cache(&cache->c,
3239-
cache->argtype, -1,
3240-
fnmcxt, false);
3241-
if (cache->c.typcat != TYPECAT_COMPOSITE &&
3242-
cache->c.typcat != TYPECAT_COMPOSITE_DOMAIN)
3243-
ereport(ERROR,
3244-
(errcode(ERRCODE_DATATYPE_MISMATCH),
3245-
errmsg("first argument of %s must be a row type",
3246-
funcname)));
3247-
}
3296+
get_record_type_from_argument(fcinfo, funcname, cache);
32483297
else
3249-
{
3250-
/*
3251-
* json{b}_to_record case: result type is specified by calling
3252-
* query. Here it is syntactically impossible to specify the
3253-
* target type as domain-over-composite.
3254-
*/
3255-
TupleDesc tupdesc;
3256-
MemoryContext old_cxt;
3257-
3258-
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
3259-
ereport(ERROR,
3260-
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
3261-
errmsg("function returning record called in context "
3262-
"that cannot accept type record"),
3263-
errhint("Try calling the function in the FROM clause "
3264-
"using a column definition list.")));
3265-
3266-
Assert(tupdesc);
3267-
cache->argtype = tupdesc->tdtypeid;
3268-
3269-
/* Save identified tupdesc */
3270-
old_cxt = MemoryContextSwitchTo(fnmcxt);
3271-
cache->c.io.composite.tupdesc = CreateTupleDescCopy(tupdesc);
3272-
cache->c.io.composite.base_typid = tupdesc->tdtypeid;
3273-
cache->c.io.composite.base_typmod = tupdesc->tdtypmod;
3274-
MemoryContextSwitchTo(old_cxt);
3275-
}
3298+
get_record_type_from_query(fcinfo, funcname, cache);
32763299
}
32773300

32783301
/* Collect record arg if we have one */
3279-
if (have_record_arg && !PG_ARGISNULL(0))
3302+
if (!have_record_arg)
3303+
rec = NULL; /* it's json{b}_to_record() */
3304+
else if (!PG_ARGISNULL(0))
32803305
{
32813306
rec = PG_GETARG_HEAPTUPLEHEADER(0);
32823307

32833308
/*
32843309
* When declared arg type is RECORD, identify actual record type from
3285-
* the tuple itself. Note the lookup_rowtype_tupdesc call in
3286-
* update_cached_tupdesc will fail if we're unable to do this.
3310+
* the tuple itself.
32873311
*/
32883312
if (cache->argtype == RECORDOID)
32893313
{
@@ -3292,8 +3316,21 @@ populate_record_worker(FunctionCallInfo fcinfo, const char *funcname,
32923316
}
32933317
}
32943318
else
3319+
{
32953320
rec = NULL;
32963321

3322+
/*
3323+
* When declared arg type is RECORD, identify actual record type from
3324+
* calling query, or fail if we can't.
3325+
*/
3326+
if (cache->argtype == RECORDOID)
3327+
{
3328+
get_record_type_from_query(fcinfo, funcname, cache);
3329+
/* This can't change argtype, which is important for next time */
3330+
Assert(cache->argtype == RECORDOID);
3331+
}
3332+
}
3333+
32973334
/* If no JSON argument, just return the record (if any) unchanged */
32983335
if (PG_ARGISNULL(json_arg_num))
32993336
{
@@ -3517,7 +3554,7 @@ json_to_recordset(PG_FUNCTION_ARGS)
35173554
static void
35183555
populate_recordset_record(PopulateRecordsetState *state, JsObject *obj)
35193556
{
3520-
PopulateRecordsetCache *cache = state->cache;
3557+
PopulateRecordCache *cache = state->cache;
35213558
HeapTupleHeader tuphead;
35223559
HeapTupleData tuple;
35233560

@@ -3559,7 +3596,7 @@ populate_recordset_worker(FunctionCallInfo fcinfo, const char *funcname,
35593596
ReturnSetInfo *rsi;
35603597
MemoryContext old_cxt;
35613598
HeapTupleHeader rec;
3562-
PopulateRecordsetCache *cache = fcinfo->flinfo->fn_extra;
3599+
PopulateRecordCache *cache = fcinfo->flinfo->fn_extra;
35633600
PopulateRecordsetState *state;
35643601

35653602
rsi = (ReturnSetInfo *) fcinfo->resultinfo;
@@ -3585,60 +3622,21 @@ populate_recordset_worker(FunctionCallInfo fcinfo, const char *funcname,
35853622
cache->fn_mcxt = fcinfo->flinfo->fn_mcxt;
35863623

35873624
if (have_record_arg)
3588-
{
3589-
/*
3590-
* json{b}_populate_recordset case: result type will be same as
3591-
* first argument's.
3592-
*/
3593-
cache->argtype = get_fn_expr_argtype(fcinfo->flinfo, 0);
3594-
prepare_column_cache(&cache->c,
3595-
cache->argtype, -1,
3596-
cache->fn_mcxt, false);
3597-
if (cache->c.typcat != TYPECAT_COMPOSITE &&
3598-
cache->c.typcat != TYPECAT_COMPOSITE_DOMAIN)
3599-
ereport(ERROR,
3600-
(errcode(ERRCODE_DATATYPE_MISMATCH),
3601-
errmsg("first argument of %s must be a row type",
3602-
funcname)));
3603-
}
3625+
get_record_type_from_argument(fcinfo, funcname, cache);
36043626
else
3605-
{
3606-
/*
3607-
* json{b}_to_recordset case: result type is specified by calling
3608-
* query. Here it is syntactically impossible to specify the
3609-
* target type as domain-over-composite.
3610-
*/
3611-
TupleDesc tupdesc;
3612-
3613-
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
3614-
ereport(ERROR,
3615-
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
3616-
errmsg("function returning record called in context "
3617-
"that cannot accept type record"),
3618-
errhint("Try calling the function in the FROM clause "
3619-
"using a column definition list.")));
3620-
3621-
Assert(tupdesc);
3622-
cache->argtype = tupdesc->tdtypeid;
3623-
3624-
/* Save identified tupdesc */
3625-
old_cxt = MemoryContextSwitchTo(cache->fn_mcxt);
3626-
cache->c.io.composite.tupdesc = CreateTupleDescCopy(tupdesc);
3627-
cache->c.io.composite.base_typid = tupdesc->tdtypeid;
3628-
cache->c.io.composite.base_typmod = tupdesc->tdtypmod;
3629-
MemoryContextSwitchTo(old_cxt);
3630-
}
3627+
get_record_type_from_query(fcinfo, funcname, cache);
36313628
}
36323629

36333630
/* Collect record arg if we have one */
3634-
if (have_record_arg && !PG_ARGISNULL(0))
3631+
if (!have_record_arg)
3632+
rec = NULL; /* it's json{b}_to_recordset() */
3633+
else if (!PG_ARGISNULL(0))
36353634
{
36363635
rec = PG_GETARG_HEAPTUPLEHEADER(0);
36373636

36383637
/*
36393638
* When declared arg type is RECORD, identify actual record type from
3640-
* the tuple itself. Note the lookup_rowtype_tupdesc call in
3641-
* update_cached_tupdesc will fail if we're unable to do this.
3639+
* the tuple itself.
36423640
*/
36433641
if (cache->argtype == RECORDOID)
36443642
{
@@ -3647,8 +3645,21 @@ populate_recordset_worker(FunctionCallInfo fcinfo, const char *funcname,
36473645
}
36483646
}
36493647
else
3648+
{
36503649
rec = NULL;
36513650

3651+
/*
3652+
* When declared arg type is RECORD, identify actual record type from
3653+
* calling query, or fail if we can't.
3654+
*/
3655+
if (cache->argtype == RECORDOID)
3656+
{
3657+
get_record_type_from_query(fcinfo, funcname, cache);
3658+
/* This can't change argtype, which is important for next time */
3659+
Assert(cache->argtype == RECORDOID);
3660+
}
3661+
}
3662+
36523663
/* if the json is null send back an empty set */
36533664
if (PG_ARGISNULL(json_arg_num))
36543665
PG_RETURN_NULL();

src/test/regress/expected/json.out

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1744,13 +1744,21 @@ SELECT rec FROM json_populate_record(
17441744

17451745
-- anonymous record type
17461746
SELECT json_populate_record(null::record, '{"x": 0, "y": 1}');
1747-
ERROR: record type has not been registered
1747+
ERROR: could not determine row type for result of json_populate_record
1748+
HINT: Provide a non-null record argument, or call the function in the FROM clause using a column definition list.
17481749
SELECT json_populate_record(row(1,2), '{"f1": 0, "f2": 1}');
17491750
json_populate_record
17501751
----------------------
17511752
(0,1)
17521753
(1 row)
17531754

1755+
SELECT * FROM
1756+
json_populate_record(null::record, '{"x": 776}') AS (x int, y int);
1757+
x | y
1758+
-----+---
1759+
776 |
1760+
(1 row)
1761+
17541762
-- composite domain
17551763
SELECT json_populate_record(null::j_ordered_pair, '{"x": 0, "y": 1}');
17561764
json_populate_record
@@ -1834,7 +1842,8 @@ select * from json_populate_recordset(row('def',99,null)::jpop,'[{"a":[100,200,3
18341842

18351843
-- anonymous record type
18361844
SELECT json_populate_recordset(null::record, '[{"x": 0, "y": 1}]');
1837-
ERROR: record type has not been registered
1845+
ERROR: could not determine row type for result of json_populate_recordset
1846+
HINT: Provide a non-null record argument, or call the function in the FROM clause using a column definition list.
18381847
SELECT json_populate_recordset(row(1,2), '[{"f1": 0, "f2": 1}]');
18391848
json_populate_recordset
18401849
-------------------------
@@ -1851,9 +1860,17 @@ FROM (VALUES (1),(2)) v(i);
18511860
2 | (2,43)
18521861
(4 rows)
18531862

1863+
SELECT * FROM
1864+
json_populate_recordset(null::record, '[{"x": 776}]') AS (x int, y int);
1865+
x | y
1866+
-----+---
1867+
776 |
1868+
(1 row)
1869+
18541870
-- empty array is a corner case
18551871
SELECT json_populate_recordset(null::record, '[]');
1856-
ERROR: record type has not been registered
1872+
ERROR: could not determine row type for result of json_populate_recordset
1873+
HINT: Provide a non-null record argument, or call the function in the FROM clause using a column definition list.
18571874
SELECT json_populate_recordset(row(1,2), '[]');
18581875
json_populate_recordset
18591876
-------------------------
@@ -1864,6 +1881,12 @@ SELECT * FROM json_populate_recordset(NULL::jpop,'[]') q;
18641881
---+---+---
18651882
(0 rows)
18661883

1884+
SELECT * FROM
1885+
json_populate_recordset(null::record, '[]') AS (x int, y int);
1886+
x | y
1887+
---+---
1888+
(0 rows)
1889+
18671890
-- composite domain
18681891
SELECT json_populate_recordset(null::j_ordered_pair, '[{"x": 0, "y": 1}]');
18691892
json_populate_recordset

0 commit comments

Comments
 (0)