From a4f6f725058a5b65b0871bec9af5f1a6dff7f2e9 Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Wed, 28 Oct 2026 19:41:46 +0900 Subject: [PATCH v4 2/2] SQL/JSON: fix error handling when coercing to a domain fails softly SQL/JSON query functions could throw an error, or report the wrong one, in cases where the ON ERROR or ON EMPTY clause said what to do instead. This happened when the RETURNING type, or a DEFAULT expression, involved a domain: if the value failed to convert to the domain's base type, that failure was saved to be handled by the ON ERROR behavior, but the domain's constraints were then checked anyway, against the dummy NULL left behind by the failed conversion. A CHECK expression can throw a hard error on that NULL, which ON ERROR cannot catch, and a NOT NULL constraint reports a violation that overwrites the saved error, so the error message ends up describing the wrong problem. For example, this failed with "division by zero" instead of returning 11: CREATE DOMAIN d AS int CHECK (1 / coalesce(VALUE, 0) = 0); SELECT JSON_VALUE(jsonb '"x"', '$' RETURNING d DEFAULT 11 ON ERROR); Fix by stopping as soon as a soft error has been saved, in each of the places that kept going: * The domain constraint checks of a CoerceToDomain in an ON ERROR or ON EMPTY DEFAULT expression, which is the only case where they are evaluated softly. ExecInitCoerceToDomain now emits a jump before each constraint check that skips the remaining checks if an error has already been saved. * populate_domain, called from json_populate_type when coercing to a domain. Return as soon as the conversion to the base type fails, instead of going on to check the domain's constraints. * populate_record, likewise. Stop populating the remaining columns as soon as one of them fails. * ExecEvalJsonCoercion. Return at once if the DEFAULT expression whose result it would coerce has already failed, and leave throwing the error to the COERCION_FINISH step that follows. Reported-by: jian he Author: jian he Discussion: https://postgr.es/m/CACJufxHDK39zF+1zZF6prH_wtn1SDt74_mrqYk1Oo82cCu44ng@mail.gmail.com Backpatch-through: 17 --- src/backend/executor/execExpr.c | 23 ++++++++ src/backend/executor/execExprInterp.c | 14 +++++ src/backend/utils/adt/jsonfuncs.c | 19 +++++++ .../regress/expected/sqljson_queryfuncs.out | 54 +++++++++++++++++++ src/test/regress/sql/sqljson_queryfuncs.sql | 18 +++++++ 5 files changed, 128 insertions(+) diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 9a7424273c0..35cec43fb3d 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -3520,6 +3520,7 @@ ExecInitCoerceToDomain(ExprEvalStep *scratch, CoerceToDomain *ctest, Datum *domainval = NULL; bool *domainnull = NULL; ListCell *l; + List *skipjumps = NIL; scratch->d.domaincheck.resulttype = ctest->resulttype; /* we'll allocate workspace only if needed */ @@ -3570,6 +3571,24 @@ ExecInitCoerceToDomain(ExprEvalStep *scratch, CoerceToDomain *ctest, scratch->d.domaincheck.constraintname = con->name; + /* + * If this CoerceToDomain is being evaluated softly and its argument, + * or a constraint checked before this one, has already failed, the + * result is going to be discarded, so don't check the remaining + * constraints against it; doing so might throw a hard error or + * overwrite the saved error. + */ + if (state->escontext != NULL) + { + ExprEvalStep jump = {0}; + + jump.opcode = EEOP_JUMP_IF_NULL; + jump.resnull = &state->escontext->error_occurred; + jump.d.jump.jumpdone = -1; /* set below */ + skipjumps = lappend_int(skipjumps, state->steps_len); + ExprEvalPushStep(state, &jump); + } + switch (con->constrainttype) { case DOM_CONSTRAINT_NOTNULL: @@ -3650,6 +3669,10 @@ ExecInitCoerceToDomain(ExprEvalStep *scratch, CoerceToDomain *ctest, break; } } + + /* Now that we know where the constraint steps end, fix up the jumps. */ + foreach_int(jumpno, skipjumps) + state->steps[jumpno].d.jump.jumpdone = state->steps_len; } /* diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index 397219f7a3a..7ccd48d2300 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -5188,6 +5188,20 @@ ExecEvalJsonCoercion(ExprState *state, ExprEvalStep *op, { ErrorSaveContext *escontext = op->d.jsonexpr_coercion.escontext; + /* + * If an error was saved while evaluating the ON ERROR or ON EMPTY + * DEFAULT expression whose result we are to coerce, that result is going + * to be discarded when the following COERCION_FINISH step throws the + * error. Don't coerce it; doing so might throw a hard error or + * overwrite the saved error. + */ + if (SOFT_ERROR_OCCURRED(escontext)) + { + *op->resnull = true; + *op->resvalue = (Datum) 0; + return; + } + /* * Prepare to call json_populate_type() to coerce the boolean result of * JSON_EXISTS_OP to the target type. If the target type is integer or a diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c index 274ed9d3184..1d3c8c0865a 100644 --- a/src/backend/utils/adt/jsonfuncs.c +++ b/src/backend/utils/adt/jsonfuncs.c @@ -3233,6 +3233,17 @@ populate_domain(DomainIOData *io, colname, mcxt, PointerGetDatum(NULL), jsv, isnull, escontext, omit_quotes); Assert(!*isnull || SOFT_ERROR_OCCURRED(escontext)); + + /* + * If converting to the base type failed softly, the caller is going + * to discard the result, so don't check the domain's constraints on + * it; doing so might throw a hard error or overwrite the saved error. + */ + if (SOFT_ERROR_OCCURRED(escontext)) + { + *isnull = true; + return (Datum) 0; + } } if (!domain_check_safe(res, *isnull, typid, &io->domain_info, mcxt, @@ -3616,6 +3627,14 @@ populate_record(TupleDesc tupdesc, &nulls[i], escontext, false); + + /* + * Once a column has failed softly, the caller is going to discard the + * whole tuple, so don't populate the remaining columns; doing so + * might throw a hard error or overwrite the saved error. + */ + if (SOFT_ERROR_OCCURRED(escontext)) + break; } res = heap_form_tuple(tupdesc, values, nulls); diff --git a/src/test/regress/expected/sqljson_queryfuncs.out b/src/test/regress/expected/sqljson_queryfuncs.out index a99429a08e6..209fb3382c3 100644 --- a/src/test/regress/expected/sqljson_queryfuncs.out +++ b/src/test/regress/expected/sqljson_queryfuncs.out @@ -1532,6 +1532,60 @@ DETAIL: invalid input syntax for type integer: "a0" SELECT JSON_QUERY('"a"', '$' RETURNING int DEFAULT (('a' || (random() * 0)::int)::int + 0) ON ERROR); ERROR: could not coerce ON ERROR expression (DEFAULT) to the RETURNING type DETAIL: invalid input syntax for type integer: "a0" +-- error. should report that the DEFAULT expression cannot be coerced to the +-- RETURNING type, not a NOT NULL violation +SELECT JSON_VALUE(jsonb '{"a": 1}', '$' RETURNING sqljsonb_int_not_null DEFAULT concat('y') ON ERROR); +ERROR: could not coerce ON ERROR expression (DEFAULT) to the RETURNING type +DETAIL: invalid input syntax for type integer: "y" +-- domain constraints must not be checked once a soft error has already occurred +CREATE DOMAIN queryfuncs_int AS int CHECK (1 / coalesce(VALUE, 0) = 0); +SELECT JSON_VALUE(jsonb '"x"', '$' RETURNING int DEFAULT 11 ON ERROR); + json_value +------------ + 11 +(1 row) + +SELECT JSON_QUERY(jsonb '["x"]', '$' RETURNING int[] DEFAULT '{11}' ON ERROR); + json_query +------------ + {11} +(1 row) + +SELECT JSON_VALUE(jsonb '"x"', '$' RETURNING queryfuncs_int DEFAULT 11 ON ERROR); + json_value +------------ + 11 +(1 row) + +SELECT JSON_QUERY(jsonb '["x"]', '$' RETURNING queryfuncs_int[] DEFAULT '{11}' ON ERROR); + json_query +------------ + {11} +(1 row) + +CREATE TYPE compcoalesce AS (a int, b queryfuncs_int); +SELECT JSON_QUERY(jsonb '{"a": "x"}', '$' RETURNING compcoalesce); + json_query +------------ + +(1 row) + +SELECT JSON_QUERY(jsonb '{"a": 1, "b": 11}', '$' RETURNING compcoalesce); + json_query +------------ + (1,11) +(1 row) + +SELECT JSON_QUERY(jsonb '{"a": 1, "b": "B"}', '$' RETURNING compcoalesce); + json_query +------------ + +(1 row) + +SELECT JSON_QUERY(jsonb '{"a": 1, "b": 0}', '$' RETURNING compcoalesce); -- error +ERROR: division by zero +DROP TYPE compcoalesce; +DROP DOMAIN queryfuncs_int; -- Test implicit coercion to a domain over fixed-length type specified in -- RETURNING CREATE DOMAIN queryfuncs_char2 AS char(2); diff --git a/src/test/regress/sql/sqljson_queryfuncs.sql b/src/test/regress/sql/sqljson_queryfuncs.sql index 9b81429b4bc..d4ba4c710db 100644 --- a/src/test/regress/sql/sqljson_queryfuncs.sql +++ b/src/test/regress/sql/sqljson_queryfuncs.sql @@ -516,6 +516,24 @@ SELECT JSON_VALUE('"a"', '$' RETURNING int DEFAULT (NULL::sqljsonb_int_not_null SELECT JSON_QUERY('"a"', '$' RETURNING int DEFAULT (NULL::sqljsonb_int_not_null + 0) ON ERROR); SELECT JSON_VALUE('"a"', '$' RETURNING int DEFAULT (('a' || (random() * 0)::int)::int + 0) ON ERROR); SELECT JSON_QUERY('"a"', '$' RETURNING int DEFAULT (('a' || (random() * 0)::int)::int + 0) ON ERROR); +-- error. should report that the DEFAULT expression cannot be coerced to the +-- RETURNING type, not a NOT NULL violation +SELECT JSON_VALUE(jsonb '{"a": 1}', '$' RETURNING sqljsonb_int_not_null DEFAULT concat('y') ON ERROR); + +-- domain constraints must not be checked once a soft error has already occurred +CREATE DOMAIN queryfuncs_int AS int CHECK (1 / coalesce(VALUE, 0) = 0); +SELECT JSON_VALUE(jsonb '"x"', '$' RETURNING int DEFAULT 11 ON ERROR); +SELECT JSON_QUERY(jsonb '["x"]', '$' RETURNING int[] DEFAULT '{11}' ON ERROR); +SELECT JSON_VALUE(jsonb '"x"', '$' RETURNING queryfuncs_int DEFAULT 11 ON ERROR); +SELECT JSON_QUERY(jsonb '["x"]', '$' RETURNING queryfuncs_int[] DEFAULT '{11}' ON ERROR); + +CREATE TYPE compcoalesce AS (a int, b queryfuncs_int); +SELECT JSON_QUERY(jsonb '{"a": "x"}', '$' RETURNING compcoalesce); +SELECT JSON_QUERY(jsonb '{"a": 1, "b": 11}', '$' RETURNING compcoalesce); +SELECT JSON_QUERY(jsonb '{"a": 1, "b": "B"}', '$' RETURNING compcoalesce); +SELECT JSON_QUERY(jsonb '{"a": 1, "b": 0}', '$' RETURNING compcoalesce); -- error +DROP TYPE compcoalesce; +DROP DOMAIN queryfuncs_int; -- Test implicit coercion to a domain over fixed-length type specified in -- RETURNING -- 2.47.3