On Mon, May 19, 2025 at 9:09 AM jian he <jian.universal...@gmail.com> wrote: > > hi. > > somehow, I accidentally saw the TODOs (commits [3]) on jsonb.c and json.c > for functions: to_json_is_immutable and to_jsonb_is_immutable. > The attached patch is to finalize these TODOs. > > per coverage [1], [2], there was zero coverage for these two functions. > so I also added extensive tests on it. >
I didn't include "utils/rel.h", so v1-0001 won't compile. The tests were overly verbose, so I removed some unnecessary ones to simplify them.
From 1c1a162ee9294475ad8beb3afd732fc6ae073b6e Mon Sep 17 00:00:00 2001 From: jian he <jian.universal...@gmail.com> Date: Wed, 28 May 2025 14:50:23 +0800 Subject: [PATCH v2 1/1] enhance json_array, json_object expression is immutable or not this will make to_json_is_immutable, to_jsonb_is_immutable recurse to composite data type or array type elements. also add extensive regress tests for it. discussion: https://postgr.es/m/CACJufxFz%3DOsXQdsMJ-cqoqspD9aJrwntsQP-U2A-UaV_M%2B-S9g%40mail.gmail.com --- src/backend/optimizer/util/clauses.c | 10 ++- src/backend/utils/adt/json.c | 68 ++++++++++++++---- src/backend/utils/adt/jsonb.c | 69 +++++++++++++++---- src/include/utils/json.h | 2 +- src/include/utils/jsonb.h | 2 +- src/test/regress/expected/sqljson.out | 99 +++++++++++++++++++++++++++ src/test/regress/sql/sqljson.sql | 62 +++++++++++++++++ 7 files changed, 281 insertions(+), 31 deletions(-) diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 26a3e050086..f69c68ee15c 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -406,10 +406,14 @@ contain_mutable_functions_walker(Node *node, void *context) foreach(lc, ctor->args) { Oid typid = exprType(lfirst(lc)); + bool contain_mutable = false; - if (is_jsonb ? - !to_jsonb_is_immutable(typid) : - !to_json_is_immutable(typid)) + if (is_jsonb) + to_jsonb_is_immutable(typid, &contain_mutable); + else + to_json_is_immutable(typid, &contain_mutable); + + if(contain_mutable) return true; } diff --git a/src/backend/utils/adt/json.c b/src/backend/utils/adt/json.c index 51452755f58..36b6920e850 100644 --- a/src/backend/utils/adt/json.c +++ b/src/backend/utils/adt/json.c @@ -13,6 +13,7 @@ */ #include "postgres.h" +#include "access/relation.h" #include "catalog/pg_proc.h" #include "catalog/pg_type.h" #include "common/hashfn.h" @@ -28,6 +29,7 @@ #include "utils/json.h" #include "utils/jsonfuncs.h" #include "utils/lsyscache.h" +#include "utils/rel.h" #include "utils/typcache.h" @@ -692,15 +694,56 @@ row_to_json_pretty(PG_FUNCTION_ARGS) /* * Is the given type immutable when coming out of a JSON context? * - * At present, datetimes are all considered mutable, because they - * depend on timezone. XXX we should also drill down into objects - * and arrays, but do not. + * At present, datetimes are all considered mutable, because they depend on + * timezone. */ -bool -to_json_is_immutable(Oid typoid) +void +to_json_is_immutable(Oid typoid, bool *contain_mutable) { + char att_typtype = get_typtype(typoid); JsonTypeCategory tcategory; Oid outfuncoid; + Oid att_typelem; + + /* since this function recurses, it could be driven to stack overflow */ + check_stack_depth(); + + Assert(contain_mutable != NULL); + + if (att_typtype == TYPTYPE_DOMAIN) + to_json_is_immutable(getBaseType(typoid), contain_mutable); + else if (att_typtype == TYPTYPE_COMPOSITE) + { + /* + * For a composite type, recurse into its attributes. + */ + Relation relation; + TupleDesc tupdesc; + int i; + + relation = relation_open(get_typ_typrelid(typoid), AccessShareLock); + + tupdesc = RelationGetDescr(relation); + + for (i = 0; i < tupdesc->natts; i++) + { + Form_pg_attribute attr = TupleDescAttr(tupdesc, i); + + if (attr->attisdropped) + continue; + to_json_is_immutable(attr->atttypid, contain_mutable); + } + relation_close(relation, AccessShareLock); + } + else if (att_typtype == TYPTYPE_RANGE) + { + to_json_is_immutable(get_range_subtype(typoid), contain_mutable); + } + else if (OidIsValid((att_typelem = get_element_type(typoid)))) + { + /* recurse into array element type */ + to_json_is_immutable(att_typelem, contain_mutable); + } json_categorize_type(typoid, false, &tcategory, &outfuncoid); @@ -710,26 +753,25 @@ to_json_is_immutable(Oid typoid) case JSONTYPE_JSON: case JSONTYPE_JSONB: case JSONTYPE_NULL: - return true; + break; case JSONTYPE_DATE: case JSONTYPE_TIMESTAMP: case JSONTYPE_TIMESTAMPTZ: - return false; + *contain_mutable = true; + break; case JSONTYPE_ARRAY: - return false; /* TODO recurse into elements */ - case JSONTYPE_COMPOSITE: - return false; /* TODO recurse into fields */ + break; case JSONTYPE_NUMERIC: case JSONTYPE_CAST: case JSONTYPE_OTHER: - return func_volatile(outfuncoid) == PROVOLATILE_IMMUTABLE; + if (func_volatile(outfuncoid) != PROVOLATILE_IMMUTABLE) + *contain_mutable = true; + break; } - - return false; /* not reached */ } /* diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c index da94d424d61..92d77f9c20f 100644 --- a/src/backend/utils/adt/jsonb.c +++ b/src/backend/utils/adt/jsonb.c @@ -13,6 +13,7 @@ #include "postgres.h" #include "access/htup_details.h" +#include "access/relation.h" #include "catalog/pg_proc.h" #include "catalog/pg_type.h" #include "funcapi.h" @@ -23,6 +24,7 @@ #include "utils/jsonb.h" #include "utils/jsonfuncs.h" #include "utils/lsyscache.h" +#include "utils/rel.h" #include "utils/typcache.h" typedef struct JsonbInState @@ -1041,15 +1043,57 @@ add_jsonb(Datum val, bool is_null, JsonbInState *result, /* * Is the given type immutable when coming out of a JSONB context? * - * At present, datetimes are all considered mutable, because they - * depend on timezone. XXX we should also drill down into objects and - * arrays, but do not. + * At present, datetimes are all considered mutable, because they depend on + * timezone. */ -bool -to_jsonb_is_immutable(Oid typoid) +void +to_jsonb_is_immutable(Oid typoid, bool *contain_mutable) { + char att_typtype = get_typtype(typoid); JsonTypeCategory tcategory; Oid outfuncoid; + Oid att_typelem; + + /* since this function recurses, it could be driven to stack overflow */ + check_stack_depth(); + + Assert(contain_mutable != NULL); + + if (att_typtype == TYPTYPE_DOMAIN) + to_jsonb_is_immutable(getBaseType(typoid), contain_mutable); + else if (att_typtype == TYPTYPE_COMPOSITE) + { + /* + * For a composite type, recurse into its attributes. + */ + Relation relation; + TupleDesc tupdesc; + int i; + + relation = relation_open(get_typ_typrelid(typoid), AccessShareLock); + + tupdesc = RelationGetDescr(relation); + + for (i = 0; i < tupdesc->natts; i++) + { + Form_pg_attribute attr = TupleDescAttr(tupdesc, i); + + if (attr->attisdropped) + continue; + + to_jsonb_is_immutable(attr->atttypid, contain_mutable); + } + relation_close(relation, AccessShareLock); + } + else if (att_typtype == TYPTYPE_RANGE) + { + to_jsonb_is_immutable(get_range_subtype(typoid), contain_mutable); + } + else if (OidIsValid((att_typelem = get_element_type(typoid)))) + { + /* recurse into array element type */ + to_jsonb_is_immutable(att_typelem, contain_mutable); + } json_categorize_type(typoid, true, &tcategory, &outfuncoid); @@ -1059,26 +1103,25 @@ to_jsonb_is_immutable(Oid typoid) case JSONTYPE_BOOL: case JSONTYPE_JSON: case JSONTYPE_JSONB: - return true; + break; case JSONTYPE_DATE: case JSONTYPE_TIMESTAMP: case JSONTYPE_TIMESTAMPTZ: - return false; + *contain_mutable = true; + break; case JSONTYPE_ARRAY: - return false; /* TODO recurse into elements */ - case JSONTYPE_COMPOSITE: - return false; /* TODO recurse into fields */ + break; case JSONTYPE_NUMERIC: case JSONTYPE_CAST: case JSONTYPE_OTHER: - return func_volatile(outfuncoid) == PROVOLATILE_IMMUTABLE; + if (func_volatile(outfuncoid) != PROVOLATILE_IMMUTABLE) + *contain_mutable = true; + break; } - - return false; /* not reached */ } /* diff --git a/src/include/utils/json.h b/src/include/utils/json.h index 49bbda7ac06..d5bdbe4bbaa 100644 --- a/src/include/utils/json.h +++ b/src/include/utils/json.h @@ -22,7 +22,7 @@ extern void escape_json_with_len(StringInfo buf, const char *str, int len); extern void escape_json_text(StringInfo buf, const text *txt); extern char *JsonEncodeDateTime(char *buf, Datum value, Oid typid, const int *tzp); -extern bool to_json_is_immutable(Oid typoid); +extern void to_json_is_immutable(Oid typoid, bool *contain_mutable); extern Datum json_build_object_worker(int nargs, const Datum *args, const bool *nulls, const Oid *types, bool absent_on_null, bool unique_keys); diff --git a/src/include/utils/jsonb.h b/src/include/utils/jsonb.h index fecb33b9c67..bd5777b1ce8 100644 --- a/src/include/utils/jsonb.h +++ b/src/include/utils/jsonb.h @@ -430,7 +430,7 @@ extern Datum jsonb_set_element(Jsonb *jb, Datum *path, int path_len, JsonbValue *newval); extern Datum jsonb_get_element(Jsonb *jb, Datum *path, int npath, bool *isnull, bool as_text); -extern bool to_jsonb_is_immutable(Oid typoid); +extern void to_jsonb_is_immutable(Oid typoid, bool *contain_mutable); extern Datum jsonb_build_object_worker(int nargs, const Datum *args, const bool *nulls, const Oid *types, bool absent_on_null, bool unique_keys); diff --git a/src/test/regress/expected/sqljson.out b/src/test/regress/expected/sqljson.out index 625acf3019a..d9b61ee19e9 100644 --- a/src/test/regress/expected/sqljson.out +++ b/src/test/regress/expected/sqljson.out @@ -1109,6 +1109,105 @@ CREATE OR REPLACE VIEW public.json_array_subquery_view AS FROM ( SELECT foo.i FROM ( VALUES (1), (2), (NULL::integer), (4)) foo(i)) q(a)) AS "json_array" DROP VIEW json_array_subquery_view; +create type comp1 as (a int, b date); +create domain d2 as comp1; +create domain mydomain as timestamptz; +create type mydomainrange as range(subtype=mydomain); +create type comp3 as (a int, b mydomainrange); +create table t1(a text[], b timestamp, c timestamptz, d date, + f1 comp1[], f2 timestamp[], + f3 d2[], f4 mydomainrange[], f5 comp3); +--JSON_OBJECTAGG, JSON_ARRAYAGG is aggregate function, can not be used in index +create index xx on t1(JSON_OBJECTAGG(a: b ABSENT ON NULL WITH UNIQUE KEYS RETURNING jsonb)); +ERROR: aggregate functions are not allowed in index expressions +LINE 1: create index xx on t1(JSON_OBJECTAGG(a: b ABSENT ON NULL WIT... + ^ +create index xx on t1(JSON_OBJECTAGG(a: b ABSENT ON NULL WITH UNIQUE KEYS RETURNING json)); +ERROR: aggregate functions are not allowed in index expressions +LINE 1: create index xx on t1(JSON_OBJECTAGG(a: b ABSENT ON NULL WIT... + ^ +create index xx on t1(JSON_ARRAYAGG(a RETURNING jsonb)); +ERROR: aggregate functions are not allowed in index expressions +LINE 1: create index xx on t1(JSON_ARRAYAGG(a RETURNING jsonb)); + ^ +create index xx on t1(JSON_ARRAYAGG(a RETURNING json)); +ERROR: aggregate functions are not allowed in index expressions +LINE 1: create index xx on t1(JSON_ARRAYAGG(a RETURNING json)); + ^ +-- jsonb: create expression index via json_array +create index on t1(json_array(a returning jsonb)); --ok +create index on t1(json_array(b returning jsonb)); --error +ERROR: functions in index expression must be marked IMMUTABLE +create index on t1(json_array(c returning jsonb)); --error +ERROR: functions in index expression must be marked IMMUTABLE +create index on t1(json_array(d returning jsonb)); --error +ERROR: functions in index expression must be marked IMMUTABLE +create index on t1(json_array(f1 returning jsonb)); --error +ERROR: functions in index expression must be marked IMMUTABLE +create index on t1(json_array(f2 returning jsonb)); --error +ERROR: functions in index expression must be marked IMMUTABLE +create index on t1(json_array(f3 returning jsonb)); --error +ERROR: functions in index expression must be marked IMMUTABLE +create index on t1(json_array(f4 returning jsonb)); --error +ERROR: functions in index expression must be marked IMMUTABLE +create index on t1(json_array(f5 returning jsonb)); --error +ERROR: functions in index expression must be marked IMMUTABLE +--jsonb: create expression index via json_object +create index on t1(json_object('hello' value a returning jsonb)); --ok +create index on t1(json_object('hello' value b returning jsonb)); --error +ERROR: functions in index expression must be marked IMMUTABLE +create index on t1(json_object('hello' value c returning jsonb)); --error +ERROR: functions in index expression must be marked IMMUTABLE +create index on t1(json_object('hello' value d returning jsonb)); --error +ERROR: functions in index expression must be marked IMMUTABLE +create index on t1(json_object('hello' value f1 returning jsonb)); --error +ERROR: functions in index expression must be marked IMMUTABLE +create index on t1(json_object('hello' value f2 returning jsonb)); --error +ERROR: functions in index expression must be marked IMMUTABLE +create index on t1(json_object('hello' value f3 returning jsonb)); --error +ERROR: functions in index expression must be marked IMMUTABLE +create index on t1(json_object('hello' value f4 returning jsonb)); --error +ERROR: functions in index expression must be marked IMMUTABLE +create index on t1(json_object('hello' value f5 returning jsonb)); --error +ERROR: functions in index expression must be marked IMMUTABLE +-- data type json don't have default operator class for access method "btree" so +-- we use a generated column to test whether the JSON_ARRAY expression is +-- immutable +alter table t1 add column f10 json generated always as (json_array(a returning json)); --ok +alter table t1 add column f11 json generated always as (json_array(b returning json)); --error +ERROR: generation expression is not immutable +alter table t1 add column f11 json generated always as (json_array(c returning json)); --error +ERROR: generation expression is not immutable +alter table t1 add column f11 json generated always as (json_array(d returning json)); --error +ERROR: generation expression is not immutable +alter table t1 add column f11 json generated always as (json_array(f1 returning json)); --error +ERROR: generation expression is not immutable +alter table t1 add column f11 json generated always as (json_array(f2 returning json)); --error +ERROR: generation expression is not immutable +alter table t1 add column f11 json generated always as (json_array(f4 returning json)); --error +ERROR: generation expression is not immutable +alter table t1 add column f11 json generated always as (json_array(f5 returning json)); --error +ERROR: generation expression is not immutable +-- data type json don't have default operator class for access method "btree" so +-- we use a generated column to test whether the JSON_OBJECT expression is +-- immutable +alter table t1 add column f11 json generated always as (json_object('hello' value a returning json)); --ok +alter table t1 add column f12 json generated always as (json_object('hello' value b returning json)); --error +ERROR: generation expression is not immutable +alter table t1 add column f12 json generated always as (json_object('hello' value c returning json)); --error +ERROR: generation expression is not immutable +alter table t1 add column f12 json generated always as (json_object('hello' value d returning json)); --error +ERROR: generation expression is not immutable +alter table t1 add column f12 json generated always as (json_object('hello' value f1 returning json)); --error +ERROR: generation expression is not immutable +alter table t1 add column f12 json generated always as (json_object('hello' value f2 returning json)); --error +ERROR: generation expression is not immutable +alter table t1 add column f12 json generated always as (json_object('hello' value f3 returning json)); --error +ERROR: generation expression is not immutable +alter table t1 add column f12 json generated always as (json_object('hello' value f4 returning json)); --error +ERROR: generation expression is not immutable +alter table t1 add column f12 json generated always as (json_object('hello' value f5 returning json)); --error +ERROR: generation expression is not immutable -- IS JSON predicate SELECT NULL IS JSON; ?column? diff --git a/src/test/regress/sql/sqljson.sql b/src/test/regress/sql/sqljson.sql index 343d344d270..26a22ccab59 100644 --- a/src/test/regress/sql/sqljson.sql +++ b/src/test/regress/sql/sqljson.sql @@ -386,6 +386,68 @@ SELECT JSON_ARRAY(SELECT i FROM (VALUES (1), (2), (NULL), (4)) foo(i) RETURNING DROP VIEW json_array_subquery_view; +create type comp1 as (a int, b date); +create domain d2 as comp1; +create domain mydomain as timestamptz; +create type mydomainrange as range(subtype=mydomain); +create type comp3 as (a int, b mydomainrange); +create table t1(a text[], b timestamp, c timestamptz, d date, + f1 comp1[], f2 timestamp[], + f3 d2[], f4 mydomainrange[], f5 comp3); + +--JSON_OBJECTAGG, JSON_ARRAYAGG is aggregate function, can not be used in index +create index xx on t1(JSON_OBJECTAGG(a: b ABSENT ON NULL WITH UNIQUE KEYS RETURNING jsonb)); +create index xx on t1(JSON_OBJECTAGG(a: b ABSENT ON NULL WITH UNIQUE KEYS RETURNING json)); +create index xx on t1(JSON_ARRAYAGG(a RETURNING jsonb)); +create index xx on t1(JSON_ARRAYAGG(a RETURNING json)); + +-- jsonb: create expression index via json_array +create index on t1(json_array(a returning jsonb)); --ok +create index on t1(json_array(b returning jsonb)); --error +create index on t1(json_array(c returning jsonb)); --error +create index on t1(json_array(d returning jsonb)); --error +create index on t1(json_array(f1 returning jsonb)); --error +create index on t1(json_array(f2 returning jsonb)); --error +create index on t1(json_array(f3 returning jsonb)); --error +create index on t1(json_array(f4 returning jsonb)); --error +create index on t1(json_array(f5 returning jsonb)); --error + +--jsonb: create expression index via json_object +create index on t1(json_object('hello' value a returning jsonb)); --ok +create index on t1(json_object('hello' value b returning jsonb)); --error +create index on t1(json_object('hello' value c returning jsonb)); --error +create index on t1(json_object('hello' value d returning jsonb)); --error +create index on t1(json_object('hello' value f1 returning jsonb)); --error +create index on t1(json_object('hello' value f2 returning jsonb)); --error +create index on t1(json_object('hello' value f3 returning jsonb)); --error +create index on t1(json_object('hello' value f4 returning jsonb)); --error +create index on t1(json_object('hello' value f5 returning jsonb)); --error + +-- data type json don't have default operator class for access method "btree" so +-- we use a generated column to test whether the JSON_ARRAY expression is +-- immutable +alter table t1 add column f10 json generated always as (json_array(a returning json)); --ok +alter table t1 add column f11 json generated always as (json_array(b returning json)); --error +alter table t1 add column f11 json generated always as (json_array(c returning json)); --error +alter table t1 add column f11 json generated always as (json_array(d returning json)); --error +alter table t1 add column f11 json generated always as (json_array(f1 returning json)); --error +alter table t1 add column f11 json generated always as (json_array(f2 returning json)); --error +alter table t1 add column f11 json generated always as (json_array(f4 returning json)); --error +alter table t1 add column f11 json generated always as (json_array(f5 returning json)); --error + +-- data type json don't have default operator class for access method "btree" so +-- we use a generated column to test whether the JSON_OBJECT expression is +-- immutable +alter table t1 add column f11 json generated always as (json_object('hello' value a returning json)); --ok +alter table t1 add column f12 json generated always as (json_object('hello' value b returning json)); --error +alter table t1 add column f12 json generated always as (json_object('hello' value c returning json)); --error +alter table t1 add column f12 json generated always as (json_object('hello' value d returning json)); --error +alter table t1 add column f12 json generated always as (json_object('hello' value f1 returning json)); --error +alter table t1 add column f12 json generated always as (json_object('hello' value f2 returning json)); --error +alter table t1 add column f12 json generated always as (json_object('hello' value f3 returning json)); --error +alter table t1 add column f12 json generated always as (json_object('hello' value f4 returning json)); --error +alter table t1 add column f12 json generated always as (json_object('hello' value f5 returning json)); --error + -- IS JSON predicate SELECT NULL IS JSON; SELECT NULL IS NOT JSON; -- 2.34.1
