Обсуждение: Cache lookup error when using jsonb, json_build_object and a WITH clause
Hi all,
I found the following error when playing with jsonb and json_build_object:
=# with jsonb_data as (select * from jsonb_each('{"aa" :
"po"}'::jsonb)) select json_build_object(key,value) from jsonb_data;
ERROR: XX000: cache lookup failed for type 2147483650
LOCATION: lookup_type_cache, typcache.c:193
I would have expected the result to be the same as in the case of json:
=# with json_data as (select * from json_each('{"aa" : "po"}'::json))
select json_build_object(key,value) from json_data;json_build_object
-------------------{"aa" : "po"}
(1 row)
Regards,
--
Michael
Michael Paquier <michael.paquier@gmail.com> writes:
> I found the following error when playing with jsonb and json_build_object:
> =# with jsonb_data as (select * from jsonb_each('{"aa" :
> "po"}'::jsonb)) select json_build_object(key,value) from jsonb_data;
> ERROR: XX000: cache lookup failed for type 2147483650
> LOCATION: lookup_type_cache, typcache.c:193
The immediate problem seems to be that add_json() did not get taught
that jsonb is of TYPCATEGORY_JSON; somebody missed updating that copy
of logic that's been copied and pasted several times too many, IMNSHO.
However, now that I look at this code, it seems like it's got more
problems than that:
* it will be fooled utterly by domains over types it's interested in.
* there is nothing stopping somebody from making user-defined types
with category 'j' or 'c', which will confuse it even more.
regards, tom lane
Hi,
On 2014-05-09 21:40:07 +0900, Michael Paquier wrote:
> Hi all,
>
> I found the following error when playing with jsonb and json_build_object:
> =# with jsonb_data as (select * from jsonb_each('{"aa" :
> "po"}'::jsonb)) select json_build_object(key,value) from jsonb_data;
> ERROR: XX000: cache lookup failed for type 2147483650
> LOCATION: lookup_type_cache, typcache.c:193
>
> I would have expected the result to be the same as in the case of json:
> =# with json_data as (select * from json_each('{"aa" : "po"}'::json))
> select json_build_object(key,value) from json_data;
> json_build_object
> -------------------
> {"aa" : "po"}
> (1 row)
Whoa. There's two wierd things here:
a) "jsonb" has a typcategory 'C'. Marking a composite type. "json" has 'U'.
b) datum_to_json() thinks it's a good idea to use typcategory to decide how a type is output. Isn't that pertty
fundamentallyflawed? To detect composite types it really should look at typtype, now?
Greetings,
Andres Freund
-- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services
Andres Freund <andres@2ndquadrant.com> writes:
> Whoa. There's two wierd things here:
> a) "jsonb" has a typcategory 'C'. Marking a composite type. "json" has
> 'U'.
Yeah, that's flat out wrong. I changed it before seeing your message.
> b) datum_to_json() thinks it's a good idea to use typcategory to decide
> how a type is output. Isn't that pertty fundamentally flawed?
Indeed. I think the bit that uses TYPCATEGORY_NUMERIC as a hint to decide
whether the value can be left unquoted (assuming it looks like a number)
might be all right, but the rest of this seems pretty bogus.
regards, tom lane
I wrote:
> Andres Freund <andres@2ndquadrant.com> writes:
>> b) datum_to_json() thinks it's a good idea to use typcategory to decide
>> how a type is output. Isn't that pertty fundamentally flawed?
> Indeed. I think the bit that uses TYPCATEGORY_NUMERIC as a hint to decide
> whether the value can be left unquoted (assuming it looks like a number)
> might be all right, but the rest of this seems pretty bogus.
Actually, that would be a security hole if it weren't that CREATE TYPE for
new base types is superuser-only. Otherwise a user-defined type could
fool this logic with a malicious choice of typcategory. jsonb itself was
darn close to being a "malicious choice of typcategory" --- it's entirely
accidental that Michael's example didn't lead to a crash or even more
interesting stuff, since the code was trying to process a jsonb as though
it were a regular composite type. Other choices of typcategory could have
sent the code into the array path for something that's not an array, or
have allowed escaping to be bypassed for something that's not json, etc.
In short, there are defined ways to decide if a type is array or
composite, and this ain't how.
After further reflection I think we should lose the TYPCATEGORY_NUMERIC
business too. ruleutils.c hard-wires the set of types it will consider
to be numeric, and I see no very good reason not to do likewise here.
That will remove the need to look up the typcategory at all.
So we need to:
1. Refactor so there's only one copy of the control logic.
2. Smash domains to their base types.
3. Identify boolean, numeric, and json types by direct tests of type OID.
4. Identify array and composite types using standard methods.
Anybody see other problems to fix here?
regards, tom lane
Re: Cache lookup error when using jsonb, json_build_object and a WITH clause
От
Andrew Dunstan
Дата:
On 05/09/2014 10:07 AM, Tom Lane wrote: > After further reflection I think we should lose the TYPCATEGORY_NUMERIC > business too. ruleutils.c hard-wires the set of types it will consider > to be numeric, and I see no very good reason not to do likewise here. > That will remove the need to look up the typcategory at all. > > So we need to: > > 1. Refactor so there's only one copy of the control logic. > > 2. Smash domains to their base types. > > 3. Identify boolean, numeric, and json types by direct tests of type OID. > > 4. Identify array and composite types using standard methods. > > Anybody see other problems to fix here? I guess this is my fault. I recall some discussions when some of this was first being written about the best way to make the type based decisions, not sure at this remove whether on list or off. The origin of it is in 9.2, so if you're going to adjust it you should probably go back that far. I was aware of the domain problem, but in 2 years or so nobody has complained about it, so I guess nobody is defining domains over json. cheers andrew
On 2014-05-09 10:07:10 -0400, Tom Lane wrote: > I wrote: > > Andres Freund <andres@2ndquadrant.com> writes: > >> b) datum_to_json() thinks it's a good idea to use typcategory to decide > >> how a type is output. Isn't that pertty fundamentally flawed? > > > Indeed. I think the bit that uses TYPCATEGORY_NUMERIC as a hint to decide > > whether the value can be left unquoted (assuming it looks like a number) > > might be all right, but the rest of this seems pretty bogus. > > Actually, that would be a security hole if it weren't that CREATE TYPE for > new base types is superuser-only. Yea. I actual wonder why CREATE TYPE seems to allow createing TYPCATEGORY_COMPOSITE types - there really doesn't seem to be a usecase. > After further reflection I think we should lose the TYPCATEGORY_NUMERIC > business too. ruleutils.c hard-wires the set of types it will consider > to be numeric, and I see no very good reason not to do likewise here. > That will remove the need to look up the typcategory at all. Maybe we should expose that list or the functionality in a neater way? It's already been copied to test_decoding... > So we need to: > > 1. Refactor so there's only one copy of the control logic. > > 2. Smash domains to their base types. > > 3. Identify boolean, numeric, and json types by direct tests of type OID. > > 4. Identify array and composite types using standard methods. > > Anybody see other problems to fix here? Yea. 5) if (val_type > FirstNormalObjectId) isn't fundamentally incorrect but imo shouldn't be replaced by something like !IsCatalogType() (akin to IsCatalogRelation). At least if we decide that hunk is safe from other POVs - I am not actually 100% sure yet. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andrew Dunstan <andrew@dunslane.net> writes:
> I guess this is my fault. I recall some discussions when some of this
> was first being written about the best way to make the type based
> decisions, not sure at this remove whether on list or off. The origin of
> it is in 9.2, so if you're going to adjust it you should probably go
> back that far.
Right, will back-patch.
> I was aware of the domain problem, but in 2 years or so nobody has
> complained about it, so I guess nobody is defining domains over json.
Actually, I was more concerned about domains over other types.
For instance
regression=# create domain dd as int;
CREATE DOMAIN
regression=# select json_build_object('foo', 43);
json_build_object
-------------------
{"foo" : 43}
(1 row)
regression=# select json_build_object('foo', 43::dd);
json_build_object
-------------------
{"foo" : "43"}
(1 row)
With the attached patch, you get 43 without any quotes, which seems
right to me. However, given the lack of complaints, it might be better
to not make this behavioral change in the back branches. I'll omit
the getBaseType() call from the back-patches.
Draft HEAD patch attached.
regards, tom lane
diff --git a/src/backend/utils/adt/json.c b/src/backend/utils/adt/json.c
index 22ef402..a7364f3 100644
*** a/src/backend/utils/adt/json.c
--- b/src/backend/utils/adt/json.c
*************** typedef enum /* contexts of JSON par
*** 48,53 ****
--- 48,65 ----
JSON_PARSE_END /* saw the end of a document, expect nothing */
} JsonParseContext;
+ typedef enum /* type categories for datum_to_json */
+ {
+ JSONTYPE_NULL, /* null, so we didn't bother to identify */
+ JSONTYPE_BOOL, /* boolean (built-in types only) */
+ JSONTYPE_NUMERIC, /* numeric (ditto) */
+ JSONTYPE_JSON, /* JSON itself (and JSONB) */
+ JSONTYPE_ARRAY, /* array */
+ JSONTYPE_COMPOSITE, /* composite */
+ JSONTYPE_CAST, /* something with an explicit cast to JSON */
+ JSONTYPE_OTHER /* all else */
+ } JsonTypeCategory;
+
static inline void json_lex(JsonLexContext *lex);
static inline void json_lex_string(JsonLexContext *lex);
static inline void json_lex_number(JsonLexContext *lex, char *s, bool *num_err);
*************** static void composite_to_json(Datum comp
*** 64,75 ****
bool use_line_feeds);
static void array_dim_to_json(StringInfo result, int dim, int ndims, int *dims,
Datum *vals, bool *nulls, int *valcount,
! TYPCATEGORY tcategory, Oid typoutputfunc,
bool use_line_feeds);
static void array_to_json_internal(Datum array, StringInfo result,
bool use_line_feeds);
static void datum_to_json(Datum val, bool is_null, StringInfo result,
! TYPCATEGORY tcategory, Oid typoutputfunc, bool key_scalar);
static void add_json(Datum val, bool is_null, StringInfo result,
Oid val_type, bool key_scalar);
--- 76,91 ----
bool use_line_feeds);
static void array_dim_to_json(StringInfo result, int dim, int ndims, int *dims,
Datum *vals, bool *nulls, int *valcount,
! JsonTypeCategory tcategory, Oid outfuncoid,
bool use_line_feeds);
static void array_to_json_internal(Datum array, StringInfo result,
bool use_line_feeds);
+ static void json_categorize_type(Oid typoid,
+ JsonTypeCategory *tcategory,
+ Oid *outfuncoid);
static void datum_to_json(Datum val, bool is_null, StringInfo result,
! JsonTypeCategory tcategory, Oid outfuncoid,
! bool key_scalar);
static void add_json(Datum val, bool is_null, StringInfo result,
Oid val_type, bool key_scalar);
*************** lex_expect(JsonParseContext ctx, JsonLex
*** 143,156 ****
report_parse_error(ctx, lex);;
}
- /*
- * All the defined type categories are upper case , so use lower case here
- * so we avoid any possible clash.
- */
- /* fake type category for JSON so we can distinguish it in datum_to_json */
- #define TYPCATEGORY_JSON 'j'
- /* fake category for types that have a cast to json */
- #define TYPCATEGORY_JSON_CAST 'c'
/* chars to consider as part of an alphanumeric token */
#define JSON_ALPHANUMERIC_CHAR(c) \
(((c) >= 'a' && (c) <= 'z') || \
--- 159,164 ----
*************** extract_mb_char(char *s)
*** 1219,1232 ****
}
/*
! * Turn a scalar Datum into JSON, appending the string to "result".
*
! * Hand off a non-scalar datum to composite_to_json or array_to_json_internal
! * as appropriate.
*/
static void
datum_to_json(Datum val, bool is_null, StringInfo result,
! TYPCATEGORY tcategory, Oid typoutputfunc, bool key_scalar)
{
char *outputstr;
text *jsontext;
--- 1227,1321 ----
}
/*
! * Determine how we want to print values of a given type in datum_to_json.
*
! * 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.
! */
! static void
! json_categorize_type(Oid typoid,
! JsonTypeCategory *tcategory,
! Oid *outfuncoid)
! {
! bool typisvarlena;
!
! /* Look through any domain */
! typoid = getBaseType(typoid);
!
! /* We'll usually need to return the type output function */
! getTypeOutputInfo(typoid, outfuncoid, &typisvarlena);
!
! /* Check for known types */
! switch (typoid)
! {
! case BOOLOID:
! *tcategory = JSONTYPE_BOOL;
! break;
!
! case INT2OID:
! case INT4OID:
! case INT8OID:
! case FLOAT4OID:
! case FLOAT8OID:
! case NUMERICOID:
! *tcategory = JSONTYPE_NUMERIC;
! break;
!
! case JSONOID:
! case JSONBOID:
! *tcategory = JSONTYPE_JSON;
! break;
!
! default:
! /* Check for arrays and composites */
! if (OidIsValid(get_element_type(typoid)))
! *tcategory = JSONTYPE_ARRAY;
! else if (type_is_rowtype(typoid))
! *tcategory = JSONTYPE_COMPOSITE;
! else
! {
! /* It's probably the general case ... */
! *tcategory = JSONTYPE_OTHER;
! /* but let's look for a cast to json, if it's not built-in */
! if (typoid >= FirstNormalObjectId)
! {
! HeapTuple tuple;
!
! tuple = SearchSysCache2(CASTSOURCETARGET,
! ObjectIdGetDatum(typoid),
! ObjectIdGetDatum(JSONOID));
! if (HeapTupleIsValid(tuple))
! {
! Form_pg_cast castForm = (Form_pg_cast) GETSTRUCT(tuple);
!
! if (castForm->castmethod == COERCION_METHOD_FUNCTION)
! {
! *tcategory = JSONTYPE_CAST;
! *outfuncoid = castForm->castfunc;
! }
!
! ReleaseSysCache(tuple);
! }
! }
! }
! break;
! }
! }
!
! /*
! * 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.
! *
! * 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(Datum val, bool is_null, StringInfo result,
! JsonTypeCategory tcategory, Oid outfuncoid,
! bool key_scalar)
{
char *outputstr;
text *jsontext;
*************** datum_to_json(Datum val, bool is_null, S
*** 1239,1260 ****
return;
}
switch (tcategory)
{
! case TYPCATEGORY_ARRAY:
array_to_json_internal(val, result, false);
break;
! case TYPCATEGORY_COMPOSITE:
composite_to_json(val, result, false);
break;
! case TYPCATEGORY_BOOLEAN:
! if (!key_scalar)
! appendStringInfoString(result, DatumGetBool(val) ? "true" : "false");
else
! escape_json(result, DatumGetBool(val) ? "true" : "false");
break;
! case TYPCATEGORY_NUMERIC:
! outputstr = OidOutputFunctionCall(typoutputfunc, val);
if (key_scalar)
{
/* always quote keys */
--- 1328,1359 ----
return;
}
+ if (key_scalar &&
+ (tcategory == JSONTYPE_ARRAY ||
+ tcategory == JSONTYPE_COMPOSITE ||
+ tcategory == JSONTYPE_JSON ||
+ tcategory == JSONTYPE_CAST))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("key value must be scalar, not array, composite or json")));
+
switch (tcategory)
{
! case JSONTYPE_ARRAY:
array_to_json_internal(val, result, false);
break;
! case JSONTYPE_COMPOSITE:
composite_to_json(val, result, false);
break;
! case JSONTYPE_BOOL:
! outputstr = DatumGetBool(val) ? "true" : "false";
! if (key_scalar)
! escape_json(result, outputstr);
else
! appendStringInfoString(result, outputstr);
break;
! case JSONTYPE_NUMERIC:
! outputstr = OidOutputFunctionCall(outfuncoid, val);
if (key_scalar)
{
/* always quote keys */
*************** datum_to_json(Datum val, bool is_null, S
*** 1276,1296 ****
}
pfree(outputstr);
break;
! case TYPCATEGORY_JSON:
! /* JSON and JSONB will already be escaped */
! outputstr = OidOutputFunctionCall(typoutputfunc, val);
appendStringInfoString(result, outputstr);
pfree(outputstr);
break;
! case TYPCATEGORY_JSON_CAST:
! jsontext = DatumGetTextP(OidFunctionCall1(typoutputfunc, val));
outputstr = text_to_cstring(jsontext);
appendStringInfoString(result, outputstr);
pfree(outputstr);
pfree(jsontext);
break;
default:
! outputstr = OidOutputFunctionCall(typoutputfunc, val);
if (key_scalar && *outputstr == '\0')
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
--- 1375,1396 ----
}
pfree(outputstr);
break;
! case JSONTYPE_JSON:
! /* JSON and JSONB output will already be escaped */
! outputstr = OidOutputFunctionCall(outfuncoid, val);
appendStringInfoString(result, outputstr);
pfree(outputstr);
break;
! case JSONTYPE_CAST:
! /* outfuncoid refers to a cast function, not an output function */
! jsontext = DatumGetTextP(OidFunctionCall1(outfuncoid, val));
outputstr = text_to_cstring(jsontext);
appendStringInfoString(result, outputstr);
pfree(outputstr);
pfree(jsontext);
break;
default:
! outputstr = OidOutputFunctionCall(outfuncoid, val);
if (key_scalar && *outputstr == '\0')
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
*************** datum_to_json(Datum val, bool is_null, S
*** 1308,1315 ****
*/
static void
array_dim_to_json(StringInfo result, int dim, int ndims, int *dims, Datum *vals,
! bool *nulls, int *valcount, TYPCATEGORY tcategory,
! Oid typoutputfunc, bool use_line_feeds)
{
int i;
const char *sep;
--- 1408,1415 ----
*/
static void
array_dim_to_json(StringInfo result, int dim, int ndims, int *dims, Datum *vals,
! bool *nulls, int *valcount, JsonTypeCategory tcategory,
! Oid outfuncoid, bool use_line_feeds)
{
int i;
const char *sep;
*************** array_dim_to_json(StringInfo result, int
*** 1328,1334 ****
if (dim + 1 == ndims)
{
datum_to_json(vals[*valcount], nulls[*valcount], result, tcategory,
! typoutputfunc, false);
(*valcount)++;
}
else
--- 1428,1434 ----
if (dim + 1 == ndims)
{
datum_to_json(vals[*valcount], nulls[*valcount], result, tcategory,
! outfuncoid, false);
(*valcount)++;
}
else
*************** array_dim_to_json(StringInfo result, int
*** 1338,1344 ****
* we'll say no.
*/
array_dim_to_json(result, dim + 1, ndims, dims, vals, nulls,
! valcount, tcategory, typoutputfunc, false);
}
}
--- 1438,1444 ----
* we'll say no.
*/
array_dim_to_json(result, dim + 1, ndims, dims, vals, nulls,
! valcount, tcategory, outfuncoid, false);
}
}
*************** array_to_json_internal(Datum array, Stri
*** 1361,1372 ****
bool *nulls;
int16 typlen;
bool typbyval;
! char typalign,
! typdelim;
! Oid typioparam;
! Oid typoutputfunc;
! TYPCATEGORY tcategory;
! Oid castfunc = InvalidOid;
ndim = ARR_NDIM(v);
dim = ARR_DIMS(v);
--- 1461,1469 ----
bool *nulls;
int16 typlen;
bool typbyval;
! char typalign;
! JsonTypeCategory tcategory;
! Oid outfuncoid;
ndim = ARR_NDIM(v);
dim = ARR_DIMS(v);
*************** array_to_json_internal(Datum array, Stri
*** 1378,1421 ****
return;
}
! get_type_io_data(element_type, IOFunc_output,
! &typlen, &typbyval, &typalign,
! &typdelim, &typioparam, &typoutputfunc);
!
! if (element_type > FirstNormalObjectId)
! {
! HeapTuple tuple;
! Form_pg_cast castForm;
!
! tuple = SearchSysCache2(CASTSOURCETARGET,
! ObjectIdGetDatum(element_type),
! ObjectIdGetDatum(JSONOID));
! if (HeapTupleIsValid(tuple))
! {
! castForm = (Form_pg_cast) GETSTRUCT(tuple);
!
! if (castForm->castmethod == COERCION_METHOD_FUNCTION)
! castfunc = typoutputfunc = castForm->castfunc;
! ReleaseSysCache(tuple);
! }
! }
deconstruct_array(v, element_type, typlen, typbyval,
typalign, &elements, &nulls,
&nitems);
- if (castfunc != InvalidOid)
- tcategory = TYPCATEGORY_JSON_CAST;
- else if (element_type == RECORDOID)
- tcategory = TYPCATEGORY_COMPOSITE;
- else if (element_type == JSONOID || element_type == JSONBOID)
- tcategory = TYPCATEGORY_JSON;
- else
- tcategory = TypeCategory(element_type);
-
array_dim_to_json(result, 0, ndim, dim, elements, nulls, &count, tcategory,
! typoutputfunc, use_line_feeds);
pfree(elements);
pfree(nulls);
--- 1475,1492 ----
return;
}
! get_typlenbyvalalign(element_type,
! &typlen, &typbyval, &typalign);
! json_categorize_type(element_type,
! &tcategory, &outfuncoid);
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);
pfree(elements);
pfree(nulls);
*************** composite_to_json(Datum composite, Strin
*** 1458,1467 ****
Datum val;
bool isnull;
char *attname;
! TYPCATEGORY tcategory;
! Oid typoutput;
! bool typisvarlena;
! Oid castfunc = InvalidOid;
if (tupdesc->attrs[i]->attisdropped)
continue;
--- 1529,1536 ----
Datum val;
bool isnull;
char *attname;
! JsonTypeCategory tcategory;
! Oid outfuncoid;
if (tupdesc->attrs[i]->attisdropped)
continue;
*************** composite_to_json(Datum composite, Strin
*** 1476,1516 ****
val = heap_getattr(tuple, i + 1, tupdesc, &isnull);
! getTypeOutputInfo(tupdesc->attrs[i]->atttypid,
! &typoutput, &typisvarlena);
!
! if (tupdesc->attrs[i]->atttypid > FirstNormalObjectId)
{
! HeapTuple cast_tuple;
! Form_pg_cast castForm;
!
! cast_tuple = SearchSysCache2(CASTSOURCETARGET,
! ObjectIdGetDatum(tupdesc->attrs[i]->atttypid),
! ObjectIdGetDatum(JSONOID));
! if (HeapTupleIsValid(cast_tuple))
! {
! castForm = (Form_pg_cast) GETSTRUCT(cast_tuple);
!
! if (castForm->castmethod == COERCION_METHOD_FUNCTION)
! castfunc = typoutput = castForm->castfunc;
!
! ReleaseSysCache(cast_tuple);
! }
}
-
- if (castfunc != InvalidOid)
- tcategory = TYPCATEGORY_JSON_CAST;
- else if (tupdesc->attrs[i]->atttypid == RECORDARRAYOID)
- tcategory = TYPCATEGORY_ARRAY;
- else if (tupdesc->attrs[i]->atttypid == RECORDOID)
- tcategory = TYPCATEGORY_COMPOSITE;
- else if (tupdesc->attrs[i]->atttypid == JSONOID ||
- tupdesc->attrs[i]->atttypid == JSONBOID)
- tcategory = TYPCATEGORY_JSON;
else
! tcategory = TypeCategory(tupdesc->attrs[i]->atttypid);
! datum_to_json(val, isnull, result, tcategory, typoutput, false);
}
appendStringInfoChar(result, '}');
--- 1545,1560 ----
val = heap_getattr(tuple, i + 1, tupdesc, &isnull);
! if (isnull)
{
! tcategory = JSONTYPE_NULL;
! outfuncoid = InvalidOid;
}
else
! json_categorize_type(tupdesc->attrs[i]->atttypid,
! &tcategory, &outfuncoid);
! datum_to_json(val, isnull, result, tcategory, outfuncoid, false);
}
appendStringInfoChar(result, '}');
*************** composite_to_json(Datum composite, Strin
*** 1518,1582 ****
}
/*
! * append Json for orig_val to result. If it's a field key, make sure it's
! * of an acceptable type and is quoted.
*/
static void
! add_json(Datum val, bool is_null, StringInfo result, Oid val_type, bool key_scalar)
{
! TYPCATEGORY tcategory;
! Oid typoutput;
! bool typisvarlena;
! Oid castfunc = InvalidOid;
if (val_type == InvalidOid)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("could not determine input data type")));
!
! getTypeOutputInfo(val_type, &typoutput, &typisvarlena);
!
! if (val_type > FirstNormalObjectId)
{
! HeapTuple tuple;
! Form_pg_cast castForm;
!
! tuple = SearchSysCache2(CASTSOURCETARGET,
! ObjectIdGetDatum(val_type),
! ObjectIdGetDatum(JSONOID));
! if (HeapTupleIsValid(tuple))
! {
! castForm = (Form_pg_cast) GETSTRUCT(tuple);
!
! if (castForm->castmethod == COERCION_METHOD_FUNCTION)
! castfunc = typoutput = castForm->castfunc;
!
! ReleaseSysCache(tuple);
! }
}
-
- if (castfunc != InvalidOid)
- tcategory = TYPCATEGORY_JSON_CAST;
- else if (val_type == RECORDARRAYOID)
- tcategory = TYPCATEGORY_ARRAY;
- else if (val_type == RECORDOID)
- tcategory = TYPCATEGORY_COMPOSITE;
- else if (val_type == JSONOID || val_type == JSONBOID)
- tcategory = TYPCATEGORY_JSON;
else
! tcategory = TypeCategory(val_type);
!
! if (key_scalar &&
! (tcategory == TYPCATEGORY_ARRAY ||
! tcategory == TYPCATEGORY_COMPOSITE ||
! tcategory == TYPCATEGORY_JSON ||
! tcategory == TYPCATEGORY_JSON_CAST))
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("key value must be scalar, not array, composite or json")));
! datum_to_json(val, is_null, result, tcategory, typoutput, key_scalar);
}
/*
--- 1562,1595 ----
}
/*
! * Append JSON text for "val" to "result".
! *
! * This is just a thin wrapper around datum_to_json. If the same type will be
! * printed many times, avoid using this; better to do the json_categorize_type
! * lookups only once.
*/
static void
! add_json(Datum val, bool is_null, StringInfo result,
! Oid val_type, bool key_scalar)
{
! JsonTypeCategory tcategory;
! Oid outfuncoid;
if (val_type == InvalidOid)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("could not determine input data type")));
! if (is_null)
{
! tcategory = JSONTYPE_NULL;
! outfuncoid = InvalidOid;
}
else
! json_categorize_type(val_type,
! &tcategory, &outfuncoid);
! datum_to_json(val, is_null, result, tcategory, outfuncoid, key_scalar);
}
/*
*************** to_json(PG_FUNCTION_ARGS)
*** 1654,1704 ****
Datum val = PG_GETARG_DATUM(0);
Oid val_type = get_fn_expr_argtype(fcinfo->flinfo, 0);
StringInfo result;
! TYPCATEGORY tcategory;
! Oid typoutput;
! bool typisvarlena;
! Oid castfunc = InvalidOid;
if (val_type == InvalidOid)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("could not determine input data type")));
! result = makeStringInfo();
!
! getTypeOutputInfo(val_type, &typoutput, &typisvarlena);
!
! if (val_type > FirstNormalObjectId)
! {
! HeapTuple tuple;
! Form_pg_cast castForm;
!
! tuple = SearchSysCache2(CASTSOURCETARGET,
! ObjectIdGetDatum(val_type),
! ObjectIdGetDatum(JSONOID));
! if (HeapTupleIsValid(tuple))
! {
! castForm = (Form_pg_cast) GETSTRUCT(tuple);
!
! if (castForm->castmethod == COERCION_METHOD_FUNCTION)
! castfunc = typoutput = castForm->castfunc;
!
! ReleaseSysCache(tuple);
! }
! }
! if (castfunc != InvalidOid)
! tcategory = TYPCATEGORY_JSON_CAST;
! else if (val_type == RECORDARRAYOID)
! tcategory = TYPCATEGORY_ARRAY;
! else if (val_type == RECORDOID)
! tcategory = TYPCATEGORY_COMPOSITE;
! else if (val_type == JSONOID || val_type == JSONBOID)
! tcategory = TYPCATEGORY_JSON;
! else
! tcategory = TypeCategory(val_type);
! datum_to_json(val, false, result, tcategory, typoutput, false);
PG_RETURN_TEXT_P(cstring_to_text_with_len(result->data, result->len));
}
--- 1667,1686 ----
Datum val = PG_GETARG_DATUM(0);
Oid val_type = get_fn_expr_argtype(fcinfo->flinfo, 0);
StringInfo result;
! JsonTypeCategory tcategory;
! Oid outfuncoid;
if (val_type == InvalidOid)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("could not determine input data type")));
! json_categorize_type(val_type,
! &tcategory, &outfuncoid);
! result = makeStringInfo();
! datum_to_json(val, false, result, tcategory, outfuncoid, false);
PG_RETURN_TEXT_P(cstring_to_text_with_len(result->data, result->len));
}
*************** json_agg_transfn(PG_FUNCTION_ARGS)
*** 1714,1723 ****
oldcontext;
StringInfo state;
Datum val;
! TYPCATEGORY tcategory;
! Oid typoutput;
! bool typisvarlena;
! Oid castfunc = InvalidOid;
if (val_type == InvalidOid)
ereport(ERROR,
--- 1696,1703 ----
oldcontext;
StringInfo state;
Datum val;
! JsonTypeCategory tcategory;
! Oid outfuncoid;
if (val_type == InvalidOid)
ereport(ERROR,
*************** json_agg_transfn(PG_FUNCTION_ARGS)
*** 1734,1742 ****
{
/*
* Make this StringInfo in a context where it will persist for the
! * duration off the aggregate call. It's only needed for this initial
! * piece, as the StringInfo routines make sure they use the right
! * context to enlarge the object if necessary.
*/
oldcontext = MemoryContextSwitchTo(aggcontext);
state = makeStringInfo();
--- 1714,1722 ----
{
/*
* Make this StringInfo 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.
*/
oldcontext = MemoryContextSwitchTo(aggcontext);
state = makeStringInfo();
*************** json_agg_transfn(PG_FUNCTION_ARGS)
*** 1753,1804 ****
/* fast path for NULLs */
if (PG_ARGISNULL(1))
{
! val = (Datum) 0;
! datum_to_json(val, true, state, 0, InvalidOid, false);
PG_RETURN_POINTER(state);
}
val = PG_GETARG_DATUM(1);
! getTypeOutputInfo(val_type, &typoutput, &typisvarlena);
!
! if (val_type > FirstNormalObjectId)
! {
! HeapTuple tuple;
! Form_pg_cast castForm;
!
! tuple = SearchSysCache2(CASTSOURCETARGET,
! ObjectIdGetDatum(val_type),
! ObjectIdGetDatum(JSONOID));
! if (HeapTupleIsValid(tuple))
! {
! castForm = (Form_pg_cast) GETSTRUCT(tuple);
!
! if (castForm->castmethod == COERCION_METHOD_FUNCTION)
! castfunc = typoutput = castForm->castfunc;
!
! ReleaseSysCache(tuple);
! }
! }
!
! if (castfunc != InvalidOid)
! tcategory = TYPCATEGORY_JSON_CAST;
! else if (val_type == RECORDARRAYOID)
! tcategory = TYPCATEGORY_ARRAY;
! else if (val_type == RECORDOID)
! tcategory = TYPCATEGORY_COMPOSITE;
! else if (val_type == JSONOID || val_type == JSONBOID)
! tcategory = TYPCATEGORY_JSON;
! else
! tcategory = TypeCategory(val_type);
if (!PG_ARGISNULL(0) &&
! (tcategory == TYPCATEGORY_ARRAY || tcategory == TYPCATEGORY_COMPOSITE))
{
appendStringInfoString(state, "\n ");
}
! datum_to_json(val, false, state, tcategory, typoutput, false);
/*
* The transition type for array_agg() is declared to be "internal", which
--- 1733,1756 ----
/* fast path for NULLs */
if (PG_ARGISNULL(1))
{
! datum_to_json((Datum) 0, true, state, JSONTYPE_NULL, InvalidOid, false);
PG_RETURN_POINTER(state);
}
val = PG_GETARG_DATUM(1);
! /* XXX we do this every time?? */
! json_categorize_type(val_type,
! &tcategory, &outfuncoid);
+ /* add some whitespace if structured type and not first item */
if (!PG_ARGISNULL(0) &&
! (tcategory == JSONTYPE_ARRAY || tcategory == JSONTYPE_COMPOSITE))
{
appendStringInfoString(state, "\n ");
}
! datum_to_json(val, false, state, tcategory, outfuncoid, false);
/*
* The transition type for array_agg() is declared to be "internal", which
Andres Freund <andres@2ndquadrant.com> writes:
>> Anybody see other problems to fix here?
> Yea.
> 5)
> if (val_type > FirstNormalObjectId)
> isn't fundamentally incorrect but imo shouldn't be replaced by something
> like !IsCatalogType() (akin to IsCatalogRelation). At least if we decide
> that hunk is safe from other POVs - I am not actually 100% sure yet.
I didn't particularly like that either. The test is off-by-one, for
one thing (a type created right at OID wraparound could have
FirstNormalObjectId). However, it seems reasonable to avoid fruitless
syscache searches for builtin types, and I'm not seeing a lot of point
to wrapping this test in some macro.
regards, tom lane