From 72668013033b9e3558f7ba900aace8d153d19e11 Mon Sep 17 00:00:00 2001 From: jian he Date: Tue, 13 Oct 2026 16:18:24 +0800 Subject: [PATCH v2 1/1] Don't treat record -> json(b) conversions as immutable. The existing behavior is accidental and incorrect. Output functions are required to be stable, but only some of them are immutable; hence, output functions for container types should be treated as stable. The JSON code has not gotten this right. A recent refactoring surfaced the defect but preserved it via a special-case exception; this commit removes that exception and updates the test accordingly. This also removes the corresponding exceptions for anyarray and anycompatiblearray, which are wrong for the same reasons, but removing those exceptions is not expected to have any significant user-visible consequences. Values of type record can be constructed using ROW(), but there's no equivalent for anyarray or anycompatiblearray. Note that the change for type record is a (minor) backward compatibility break that could block upgrades; see the regression test for an example of a case that would break. Author: Robert Haas Discussion: https://postgr.es/m/CA+TgmoaiothgQrw9OtgsMzBUCnqJ2jdaGTbS6o3fkjCd+LfzWw@mail.gmail.com --- src/backend/utils/adt/json.c | 2 +- src/backend/utils/adt/jsonb.c | 2 +- src/backend/utils/adt/jsonfuncs.c | 91 +++++++++++++++++++-------- src/include/utils/jsonfuncs.h | 3 +- src/test/regress/expected/sqljson.out | 26 ++++++++ src/test/regress/sql/sqljson.sql | 8 +++ 6 files changed, 103 insertions(+), 29 deletions(-) diff --git a/src/backend/utils/adt/json.c b/src/backend/utils/adt/json.c index 28e5f3cf9c0..5aab80f8763 100644 --- a/src/backend/utils/adt/json.c +++ b/src/backend/utils/adt/json.c @@ -697,7 +697,7 @@ to_json_is_immutable(Oid typoid) { bool has_mutable = false; - json_check_mutability(typoid, false, &has_mutable); + json_check_mutability(typoid, &has_mutable); return !has_mutable; } diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c index da0061ba1b8..c24ae6b2cd2 100644 --- a/src/backend/utils/adt/jsonb.c +++ b/src/backend/utils/adt/jsonb.c @@ -1082,7 +1082,7 @@ to_jsonb_is_immutable(Oid typoid) { bool has_mutable = false; - json_check_mutability(typoid, true, &has_mutable); + json_check_mutability(typoid, &has_mutable); return !has_mutable; } diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c index adbafd0ecd3..e671298cb36 100644 --- a/src/backend/utils/adt/jsonfuncs.c +++ b/src/backend/utils/adt/jsonfuncs.c @@ -506,6 +506,7 @@ static JsonParseErrorType transform_string_values_object_field_start(void *state static JsonParseErrorType transform_string_values_array_element_start(void *state, bool isnull); static JsonParseErrorType transform_string_values_scalar(void *state, char *token, JsonTokenType tokentype); +static Oid get_json_cast_for_type(Oid typoid); /* * pg_parse_json_or_errsave @@ -6074,11 +6075,9 @@ json_categorize_type(Oid typoid, bool is_jsonb, * If any mutable function is found, *has_mutable is set to true. */ void -json_check_mutability(Oid typoid, bool is_jsonb, bool *has_mutable) +json_check_mutability(Oid typoid, bool *has_mutable) { char att_typtype = get_typtype(typoid); - JsonTypeCategory tcategory; - Oid outfuncoid; /* since this function recurses, it could be driven to stack overflow */ check_stack_depth(); @@ -6090,7 +6089,7 @@ json_check_mutability(Oid typoid, bool is_jsonb, bool *has_mutable) if (att_typtype == TYPTYPE_DOMAIN) { - json_check_mutability(getBaseType(typoid), is_jsonb, has_mutable); + json_check_mutability(getBaseType(typoid), has_mutable); return; } else if (att_typtype == TYPTYPE_COMPOSITE) @@ -6108,7 +6107,7 @@ json_check_mutability(Oid typoid, bool is_jsonb, bool *has_mutable) if (attr->attisdropped) continue; - json_check_mutability(attr->atttypid, is_jsonb, has_mutable); + json_check_mutability(attr->atttypid, has_mutable); if (*has_mutable) break; } @@ -6117,13 +6116,13 @@ json_check_mutability(Oid typoid, bool is_jsonb, bool *has_mutable) } else if (att_typtype == TYPTYPE_RANGE) { - json_check_mutability(get_range_subtype(typoid), is_jsonb, + json_check_mutability(get_range_subtype(typoid), has_mutable); return; } else if (att_typtype == TYPTYPE_MULTIRANGE) { - json_check_mutability(get_multirange_range(typoid), is_jsonb, + json_check_mutability(get_multirange_range(typoid), has_mutable); return; } @@ -6134,36 +6133,78 @@ json_check_mutability(Oid typoid, bool is_jsonb, bool *has_mutable) if (OidIsValid(att_typelem)) { /* recurse into array element type */ - json_check_mutability(att_typelem, is_jsonb, has_mutable); + json_check_mutability(att_typelem, has_mutable); return; } } - json_categorize_type(typoid, is_jsonb, &tcategory, &outfuncoid); - - switch (tcategory) + switch (typoid) { - case JSONTYPE_NULL: - case JSONTYPE_BOOL: - case JSONTYPE_NUMERIC: + case BOOLOID: + case INT2OID: + case INT4OID: + case INT8OID: + case FLOAT4OID: + case FLOAT8OID: + case NUMERICOID: + case JSONOID: + case JSONBOID: + /* known immutable */ break; - case JSONTYPE_DATE: - case JSONTYPE_TIMESTAMP: - case JSONTYPE_TIMESTAMPTZ: + case DATEOID: + case TIMESTAMPOID: + case TIMESTAMPTZOID: *has_mutable = true; break; - case JSONTYPE_JSON: - case JSONTYPE_JSONB: - case JSONTYPE_ARRAY: - case JSONTYPE_COMPOSITE: + case RECORDOID: + case ANYARRAYOID: + case ANYCOMPATIBLEARRAYOID: + + /* + * The field or element types of these pseudo-types are not known + * until runtime, so assume the worst. + */ + *has_mutable = true; break; - case JSONTYPE_CAST: - case JSONTYPE_OTHER: - if (func_volatile(outfuncoid) != PROVOLATILE_IMMUTABLE) - *has_mutable = true; + default: + { + Oid funcoid = get_json_cast_for_type(typoid); + + if (!OidIsValid(funcoid)) + { + bool typisvarlena; + + getTypeOutputInfo(typoid, &funcoid, &typisvarlena); + } + if (func_volatile(funcoid) != PROVOLATILE_IMMUTABLE) + *has_mutable = true; + } break; } } + +/* + * Return the OID of the cast function from typoid to JSON, or InvalidOid if + * there is no such cast. As a matter of policy, we only consider explicit, + * user-defined casts. This must match the lookup in json_categorize_type(): + * it is used for both json and jsonb output, a cast to jsonb is never used. + */ +static Oid +get_json_cast_for_type(Oid typoid) +{ + if (typoid >= FirstNormalObjectId) + { + Oid castfunc; + CoercionPathType ctype; + + ctype = find_coercion_pathway(JSONOID, typoid, + COERCION_EXPLICIT, + &castfunc); + if (ctype == COERCION_PATH_FUNC && OidIsValid(castfunc)) + return castfunc; + } + return InvalidOid; +} diff --git a/src/include/utils/jsonfuncs.h b/src/include/utils/jsonfuncs.h index 27713be3aeb..cc5e0244cca 100644 --- a/src/include/utils/jsonfuncs.h +++ b/src/include/utils/jsonfuncs.h @@ -83,8 +83,7 @@ typedef enum extern void json_categorize_type(Oid typoid, bool is_jsonb, JsonTypeCategory *tcategory, Oid *outfuncoid); -extern void json_check_mutability(Oid typoid, bool is_jsonb, - bool *has_mutable); +extern void json_check_mutability(Oid typoid, bool *has_mutable); extern Datum datum_to_json(Datum val, JsonTypeCategory tcategory, Oid outfuncoid); extern Datum datum_to_jsonb(Datum val, JsonTypeCategory tcategory, diff --git a/src/test/regress/expected/sqljson.out b/src/test/regress/expected/sqljson.out index 30f9e10ef0a..b649b97be40 100644 --- a/src/test/regress/expected/sqljson.out +++ b/src/test/regress/expected/sqljson.out @@ -1333,6 +1333,22 @@ create index on test_mutability(json_array(f6 returning jsonb)); -- error ERROR: functions in index expression must be marked IMMUTABLE LINE 1: create index on test_mutability(json_array(f6 returning json... ^ +create index on test_mutability(json_array(array[f6] returning jsonb)); -- error +ERROR: functions in index expression must be marked IMMUTABLE +LINE 1: create index on test_mutability(json_array(array[f6] returni... + ^ +create index on test_mutability(json_array(row(f6) returning jsonb)); -- error +ERROR: functions in index expression must be marked IMMUTABLE +LINE 1: create index on test_mutability(json_array(row(f6) returning... + ^ +create index on test_mutability(json_array(array[row(c)] returning jsonb)); -- error +ERROR: functions in index expression must be marked IMMUTABLE +LINE 1: create index on test_mutability(json_array(array[row(c)] ret... + ^ +create index on test_mutability(json_array(row(1) returning jsonb)); -- error +ERROR: functions in index expression must be marked IMMUTABLE +LINE 1: create index on test_mutability(json_array(row(1) returning ... + ^ -- jsonb: create expression index via json_object create index on test_mutability(json_object('hello' value a returning jsonb)); -- ok create index on test_mutability(json_object('hello' value b returning jsonb)); -- error @@ -1371,6 +1387,10 @@ create index on test_mutability(json_object('hello' value f6 returning jsonb)); ERROR: functions in index expression must be marked IMMUTABLE LINE 1: create index on test_mutability(json_object('hello' value f6... ^ +create index on test_mutability(json_object('hello' value row(f2) returning jsonb)); -- error +ERROR: functions in index expression must be marked IMMUTABLE +LINE 1: create index on test_mutability(json_object('hello' value ro... + ^ -- data type json doesn't have a default operator class for access method "btree" so -- we use a generated column to test whether the JSON_ARRAY expression is -- immutable @@ -1393,6 +1413,10 @@ alter table test_mutability add column f11 json generated always as (json_array( ERROR: generation expression is not immutable alter table test_mutability add column f11 json generated always as (json_array(f6 returning json)); -- error ERROR: generation expression is not immutable +alter table test_mutability add column f11 json generated always as (json_array(row(f6) returning json)); -- error +ERROR: generation expression is not immutable +alter table test_mutability add column f11 json generated always as (json_array(array[row(f6)] returning json)); -- error +ERROR: generation expression is not immutable -- data type json doesn't have a default operator class for access method "btree" so -- we use a generated column to test whether the JSON_OBJECT expression is -- immutable @@ -1415,6 +1439,8 @@ alter table test_mutability add column f12 json generated always as (json_object ERROR: generation expression is not immutable alter table test_mutability add column f12 json generated always as (json_object('hello' value f6 returning json)); -- error ERROR: generation expression is not immutable +alter table test_mutability add column f12 json generated always as (json_object('hello' value row(f2) returning json)); -- error +ERROR: generation expression is not immutable drop table test_mutability; drop domain d_comp1; drop type comp3; diff --git a/src/test/regress/sql/sqljson.sql b/src/test/regress/sql/sqljson.sql index 9fa203ccb53..e587180e7de 100644 --- a/src/test/regress/sql/sqljson.sql +++ b/src/test/regress/sql/sqljson.sql @@ -493,6 +493,10 @@ create index on test_mutability(json_array(f3 returning jsonb)); -- error create index on test_mutability(json_array(f4 returning jsonb)); -- error create index on test_mutability(json_array(f5 returning jsonb)); -- error create index on test_mutability(json_array(f6 returning jsonb)); -- error +create index on test_mutability(json_array(array[f6] returning jsonb)); -- error +create index on test_mutability(json_array(row(f6) returning jsonb)); -- error +create index on test_mutability(json_array(array[row(c)] returning jsonb)); -- error +create index on test_mutability(json_array(row(1) returning jsonb)); -- error -- jsonb: create expression index via json_object create index on test_mutability(json_object('hello' value a returning jsonb)); -- ok @@ -505,6 +509,7 @@ create index on test_mutability(json_object('hello' value f3 returning jsonb)); create index on test_mutability(json_object('hello' value f4 returning jsonb)); -- error create index on test_mutability(json_object('hello' value f5 returning jsonb)); -- error create index on test_mutability(json_object('hello' value f6 returning jsonb)); -- error +create index on test_mutability(json_object('hello' value row(f2) returning jsonb)); -- error -- data type json doesn't have a default operator class for access method "btree" so -- we use a generated column to test whether the JSON_ARRAY expression is @@ -519,6 +524,8 @@ alter table test_mutability add column f11 json generated always as (json_array( alter table test_mutability add column f11 json generated always as (json_array(f4 returning json)); -- error alter table test_mutability add column f11 json generated always as (json_array(f5 returning json)); -- error alter table test_mutability add column f11 json generated always as (json_array(f6 returning json)); -- error +alter table test_mutability add column f11 json generated always as (json_array(row(f6) returning json)); -- error +alter table test_mutability add column f11 json generated always as (json_array(array[row(f6)] returning json)); -- error -- data type json doesn't have a default operator class for access method "btree" so -- we use a generated column to test whether the JSON_OBJECT expression is @@ -533,6 +540,7 @@ alter table test_mutability add column f12 json generated always as (json_object alter table test_mutability add column f12 json generated always as (json_object('hello' value f4 returning json)); -- error alter table test_mutability add column f12 json generated always as (json_object('hello' value f5 returning json)); -- error alter table test_mutability add column f12 json generated always as (json_object('hello' value f6 returning json)); -- error +alter table test_mutability add column f12 json generated always as (json_object('hello' value row(f2) returning json)); -- error drop table test_mutability; drop domain d_comp1; -- 2.34.1