From ef5e60a9d352a97791af632e0d26a572bc88e921 Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Thu, 9 Oct 2025 01:07:59 -0400 Subject: [PATCH] Fix internal error from CollateExpr in SQL/JSON DEFAULT expressions MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit SQL/JSON functions such as JSON_VALUE could fail with "unrecognized node type" errors when a DEFAULT clause contained an explicit COLLATE expression. That happened because assign_collations_walker() could invoke exprSetCollation() on a JsonBehavior expression whose DEFAULT still contained a CollateExpr, which exprSetCollation() does not handle. For example: SELECT JSON_VALUE('{"a":1}', '$.c' RETURNING text DEFAULT 'A' COLLATE "C" ON EMPTY); Fix by validating in transformJsonBehavior() that the DEFAULT expression's collation matches the enclosing JSON expression’s collation. In exprSetCollation(), replace the recursive call on the JsonBehavior expression with an assertion that its collation already matches the target, since the parser now enforces that condition. Reported-by: Jian He Author: Jian He Reviewed-by: Amit Langote Discussion: https://postgr.es/m/CACJufxHVwYYSyiVQ6o+PsRX6zQ7rAFinh_fv1kCfTsT1xG4Zeg@mail.gmail.com Backpatch-through: 17 --- src/backend/nodes/nodeFuncs.c | 8 +-- src/backend/parser/parse_expr.c | 57 ++++++++++++++++--- .../regress/expected/collate.icu.utf8.out | 49 ++++++++++++++++ src/test/regress/sql/collate.icu.utf8.sql | 13 +++++ 4 files changed, 113 insertions(+), 14 deletions(-) diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c index 7bc823507f1..ede838cd40c 100644 --- a/src/backend/nodes/nodeFuncs.c +++ b/src/backend/nodes/nodeFuncs.c @@ -1274,12 +1274,8 @@ exprSetCollation(Node *expr, Oid collation) } break; case T_JsonBehavior: - { - JsonBehavior *behavior = (JsonBehavior *) expr; - - if (behavior->expr) - exprSetCollation(behavior->expr, collation); - } + Assert(((JsonBehavior *) expr)->expr == NULL || + exprCollation(((JsonBehavior *) expr)->expr) == collation); break; case T_NullTest: /* NullTest's result is boolean ... */ diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index 9d95c7140ee..12119f147fc 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -94,7 +94,8 @@ static Node *transformJsonFuncExpr(ParseState *pstate, JsonFuncExpr *func); static void transformJsonPassingArgs(ParseState *pstate, const char *constructName, JsonFormatType format, List *args, List **passing_values, List **passing_names); -static JsonBehavior *transformJsonBehavior(ParseState *pstate, JsonBehavior *behavior, +static JsonBehavior *transformJsonBehavior(ParseState *pstate, JsonExpr *jsexpr, + JsonBehavior *behavior, JsonBehaviorType default_behavior, JsonReturning *returning); static Node *GetJsonBehaviorConst(JsonBehaviorType btype, int location); @@ -4529,13 +4530,16 @@ transformJsonFuncExpr(ParseState *pstate, JsonFuncExpr *func) { jsexpr->returning->typid = BOOLOID; jsexpr->returning->typmod = -1; + jsexpr->collation = InvalidOid; } /* JSON_TABLE() COLUMNS can specify a non-boolean type. */ if (jsexpr->returning->typid != BOOLOID) jsexpr->use_json_coercion = true; - jsexpr->on_error = transformJsonBehavior(pstate, func->on_error, + jsexpr->on_error = transformJsonBehavior(pstate, + jsexpr, + func->on_error, JSON_BEHAVIOR_FALSE, jsexpr->returning); break; @@ -4550,6 +4554,8 @@ transformJsonFuncExpr(ParseState *pstate, JsonFuncExpr *func) ret->typmod = -1; } + jsexpr->collation = get_typcollation(jsexpr->returning->typid); + /* * Keep quotes on scalar strings by default, omitting them only if * OMIT QUOTES is specified. @@ -4566,11 +4572,15 @@ transformJsonFuncExpr(ParseState *pstate, JsonFuncExpr *func) jsexpr->use_json_coercion = true; /* Assume NULL ON EMPTY when ON EMPTY is not specified. */ - jsexpr->on_empty = transformJsonBehavior(pstate, func->on_empty, + jsexpr->on_empty = transformJsonBehavior(pstate, + jsexpr, + func->on_empty, JSON_BEHAVIOR_NULL, jsexpr->returning); /* Assume NULL ON ERROR when ON ERROR is not specified. */ - jsexpr->on_error = transformJsonBehavior(pstate, func->on_error, + jsexpr->on_error = transformJsonBehavior(pstate, + jsexpr, + func->on_error, JSON_BEHAVIOR_NULL, jsexpr->returning); break; @@ -4582,6 +4592,7 @@ transformJsonFuncExpr(ParseState *pstate, JsonFuncExpr *func) jsexpr->returning->typid = TEXTOID; jsexpr->returning->typmod = -1; } + jsexpr->collation = get_typcollation(jsexpr->returning->typid); /* * Override whatever transformJsonOutput() set these to, which @@ -4607,11 +4618,15 @@ transformJsonFuncExpr(ParseState *pstate, JsonFuncExpr *func) } /* Assume NULL ON EMPTY when ON EMPTY is not specified. */ - jsexpr->on_empty = transformJsonBehavior(pstate, func->on_empty, + jsexpr->on_empty = transformJsonBehavior(pstate, + jsexpr, + func->on_empty, JSON_BEHAVIOR_NULL, jsexpr->returning); /* Assume NULL ON ERROR when ON ERROR is not specified. */ - jsexpr->on_error = transformJsonBehavior(pstate, func->on_error, + jsexpr->on_error = transformJsonBehavior(pstate, + jsexpr, + func->on_error, JSON_BEHAVIOR_NULL, jsexpr->returning); break; @@ -4622,6 +4637,7 @@ transformJsonFuncExpr(ParseState *pstate, JsonFuncExpr *func) jsexpr->returning->typid = exprType(jsexpr->formatted_expr); jsexpr->returning->typmod = -1; } + jsexpr->collation = get_typcollation(jsexpr->returning->typid); /* * Assume EMPTY ARRAY ON ERROR when ON ERROR is not specified. @@ -4629,7 +4645,9 @@ transformJsonFuncExpr(ParseState *pstate, JsonFuncExpr *func) * ON EMPTY cannot be specified at the top level but it can be for * the individual columns. */ - jsexpr->on_error = transformJsonBehavior(pstate, func->on_error, + jsexpr->on_error = transformJsonBehavior(pstate, + jsexpr, + func->on_error, JSON_BEHAVIOR_EMPTY_ARRAY, jsexpr->returning); break; @@ -4705,7 +4723,8 @@ ValidJsonBehaviorDefaultExpr(Node *expr, void *context) * Transform a JSON BEHAVIOR clause. */ static JsonBehavior * -transformJsonBehavior(ParseState *pstate, JsonBehavior *behavior, +transformJsonBehavior(ParseState *pstate, JsonExpr *jsexpr, + JsonBehavior *behavior, JsonBehaviorType default_behavior, JsonReturning *returning) { @@ -4720,7 +4739,11 @@ transformJsonBehavior(ParseState *pstate, JsonBehavior *behavior, location = behavior->location; if (btype == JSON_BEHAVIOR_DEFAULT) { + Oid targetcoll = jsexpr->collation; + Oid exprcoll; + expr = transformExprRecurse(pstate, behavior->expr); + if (!ValidJsonBehaviorDefaultExpr(expr, NULL)) ereport(ERROR, (errcode(ERRCODE_DATATYPE_MISMATCH), @@ -4736,6 +4759,24 @@ transformJsonBehavior(ParseState *pstate, JsonBehavior *behavior, (errcode(ERRCODE_DATATYPE_MISMATCH), errmsg("DEFAULT expression must not return a set"), parser_errposition(pstate, exprLocation(expr)))); + + /* + * Reject a DEFAULT expression whose collation differs from the + * enclosing JSON expression's result collation + * (jsexpr->collation), as chosen by the RETURNING clause. + */ + exprcoll = exprCollation(expr); + if (!OidIsValid(exprcoll)) + exprcoll = get_typcollation(exprType(expr)); + if (OidIsValid(targetcoll) && OidIsValid(exprcoll) && + targetcoll != exprcoll) + ereport(ERROR, + errcode(ERRCODE_COLLATION_MISMATCH), + errmsg("the collation of DEFAULT expression conflicts with RETURNING clause"), + errdetail("\"%s\" versus \"%s\"", + get_collation_name(exprcoll), + get_collation_name(targetcoll)), + parser_errposition(pstate, exprLocation(expr))); } } diff --git a/src/test/regress/expected/collate.icu.utf8.out b/src/test/regress/expected/collate.icu.utf8.out index ef79d6f1ded..05d8b3b369e 100644 --- a/src/test/regress/expected/collate.icu.utf8.out +++ b/src/test/regress/expected/collate.icu.utf8.out @@ -2694,6 +2694,55 @@ SELECT * FROM t5 ORDER BY c ASC, a ASC; 3 | d1 | d1 (3 rows) +-- Check that DEFAULT expressions in SQL/JSON functions use the same collation +-- as the RETURNING type. Mismatched collations should raise an error. +CREATE DOMAIN d1 AS text COLLATE case_insensitive; +CREATE DOMAIN d2 AS text COLLATE "C"; +SELECT JSON_VALUE('{"a": "A"}', '$.a' RETURNING d1 DEFAULT ('C' COLLATE "C") COLLATE case_insensitive ON EMPTY) = 'a'; -- true + ?column? +---------- + t +(1 row) + +SELECT JSON_VALUE('{"a": "A"}', '$.a' RETURNING d1 DEFAULT 'C' ON EMPTY) = 'a'; -- true + ?column? +---------- + t +(1 row) + +SELECT JSON_VALUE('{"a": "A"}', '$.a' RETURNING d1 DEFAULT 'C'::d2 ON EMPTY) = 'a'; -- error +ERROR: the collation of DEFAULT expression conflicts with RETURNING clause +LINE 1: ...ON_VALUE('{"a": "A"}', '$.a' RETURNING d1 DEFAULT 'C'::d2 ON... + ^ +DETAIL: "C" versus "case_insensitive" +SELECT JSON_VALUE('{"a": "A"}', '$.a' RETURNING d1 DEFAULT 'C' COLLATE "C" ON EMPTY) = 'a'; -- error +ERROR: the collation of DEFAULT expression conflicts with RETURNING clause +LINE 1: ...ON_VALUE('{"a": "A"}', '$.a' RETURNING d1 DEFAULT 'C' COLLAT... + ^ +DETAIL: "C" versus "case_insensitive" +SELECT JSON_VALUE('{"a": "A"}', '$.c' RETURNING d1 DEFAULT 'A' ON EMPTY) = 'a'; -- true + ?column? +---------- + t +(1 row) + +SELECT JSON_VALUE('{"a": "A"}', '$.c' RETURNING d1 DEFAULT 'A' COLLATE case_insensitive ON EMPTY) = 'a'; -- true + ?column? +---------- + t +(1 row) + +SELECT JSON_VALUE('{"a": "A"}', '$.c' RETURNING d1 DEFAULT 'A'::d2 ON EMPTY) = 'a'; -- error +ERROR: the collation of DEFAULT expression conflicts with RETURNING clause +LINE 1: ...ON_VALUE('{"a": "A"}', '$.c' RETURNING d1 DEFAULT 'A'::d2 ON... + ^ +DETAIL: "C" versus "case_insensitive" +SELECT JSON_VALUE('{"a": "A"}', '$.c' RETURNING d1 DEFAULT 'A' COLLATE "C" ON EMPTY) = 'a'; -- error +ERROR: the collation of DEFAULT expression conflicts with RETURNING clause +LINE 1: ...ON_VALUE('{"a": "A"}', '$.c' RETURNING d1 DEFAULT 'A' COLLAT... + ^ +DETAIL: "C" versus "case_insensitive" +DROP DOMAIN d1, d2; -- cleanup RESET search_path; SET client_min_messages TO warning; diff --git a/src/test/regress/sql/collate.icu.utf8.sql b/src/test/regress/sql/collate.icu.utf8.sql index dbc190227d0..6f5abac0dc0 100644 --- a/src/test/regress/sql/collate.icu.utf8.sql +++ b/src/test/regress/sql/collate.icu.utf8.sql @@ -997,6 +997,19 @@ INSERT INTO t5 (a, b) values (1, 'D1'), (2, 'D2'), (3, 'd1'); -- rewriting.) SELECT * FROM t5 ORDER BY c ASC, a ASC; +-- Check that DEFAULT expressions in SQL/JSON functions use the same collation +-- as the RETURNING type. Mismatched collations should raise an error. +CREATE DOMAIN d1 AS text COLLATE case_insensitive; +CREATE DOMAIN d2 AS text COLLATE "C"; +SELECT JSON_VALUE('{"a": "A"}', '$.a' RETURNING d1 DEFAULT ('C' COLLATE "C") COLLATE case_insensitive ON EMPTY) = 'a'; -- true +SELECT JSON_VALUE('{"a": "A"}', '$.a' RETURNING d1 DEFAULT 'C' ON EMPTY) = 'a'; -- true +SELECT JSON_VALUE('{"a": "A"}', '$.a' RETURNING d1 DEFAULT 'C'::d2 ON EMPTY) = 'a'; -- error +SELECT JSON_VALUE('{"a": "A"}', '$.a' RETURNING d1 DEFAULT 'C' COLLATE "C" ON EMPTY) = 'a'; -- error +SELECT JSON_VALUE('{"a": "A"}', '$.c' RETURNING d1 DEFAULT 'A' ON EMPTY) = 'a'; -- true +SELECT JSON_VALUE('{"a": "A"}', '$.c' RETURNING d1 DEFAULT 'A' COLLATE case_insensitive ON EMPTY) = 'a'; -- true +SELECT JSON_VALUE('{"a": "A"}', '$.c' RETURNING d1 DEFAULT 'A'::d2 ON EMPTY) = 'a'; -- error +SELECT JSON_VALUE('{"a": "A"}', '$.c' RETURNING d1 DEFAULT 'A' COLLATE "C" ON EMPTY) = 'a'; -- error +DROP DOMAIN d1, d2; -- cleanup RESET search_path; -- 2.39.5