Hi.
Please see function ExecInitJsonExpr below code
``````
/*
* Steps to evaluate the ON ERROR expression; handle errors softly to
* rethrow them in COERCION_FINISH step that will be added later.
*/
saved_escontext = state->escontext;
state->escontext = escontext;
ExecInitExprRec((Expr *) jsexpr->on_error->expr,
state, resv, resnull);
state->escontext = saved_escontext;
/* Step to coerce the ON ERROR expression if needed */
if (jsexpr->on_error->coerce)
ExecInitJsonCoercion(state, jsexpr->returning, escontext,
jsexpr->omit_quotes, false,
resv, resnull);
/*
* Add a COERCION_FINISH step to check for errors that may occur when
* coercing and rethrow them.
*/
if (jsexpr->on_error->coerce ||
IsA(jsexpr->on_error->expr, CoerceViaIO) ||
IsA(jsexpr->on_error->expr, CoerceToDomain))
{
scratch->opcode = EEOP_JSONEXPR_COERCION_FINISH;
scratch->resvalue = resv;
scratch->resnull = resnull;
scratch->d.jsonexpr.jsestate = jsestate;
ExprEvalPushStep(state, scratch);
}
``````
The above code relates to SQL/JSON ON ERROR, apply the same logic to ON EMPTY.
1.
The whole expression (jsexpr->on_error->expr) could be compiled under
the ErrorSaveContext, so any part of it
may report an error softly; without a COERCION_FINISH step afterwards
unconditionally, that
error is never handled.
That is not OK if we later want to raise the error, which is the case here.
2.
By the time the coercion step from ExecInitJsonCoercion runs, a soft error
may already have occurred during expression evaluation (DEFAULT ON
ERROR, DEFAULT ON EMPTY).
ExecEvalJsonCoercion should check for that first and, if so,
set resnull and resvalue and return earlier.
I found this issue while working on
https://commitfest.postgresql.org/patch/5941.
I don't think we need to add any extra comments.
/*
* Steps to evaluate the ON ERROR expression; handle errors softly to
* rethrow them in COERCION_FINISH step that will be added later.
*/
This comment is OK even if we unconditionally add a COERCION_FINISH step.
We could executing EEOP_JSONEXPR_COERCION_FINISH only when escontext
is non-NULL.
But, adding EEOP_JSONEXPR_COERCION_FINISH unconditionally feels more
intuitive and the cost seems very little.
--
jian
https://www.enterprisedb.com/
From dbe9b3e1614721842d488ef7a8a93f91ac417439 Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
Date: Wed, 16 Sep 2026 10:08:15 +0800
Subject: [PATCH v1 1/1] fix SQL/JSON DEFAULT expression error rethrow
If the DEFAULT expression in ON ERROR or ON EMPTY clause is compiled
unconditionally in an error-safe manner, and the point of that is to rethrow the
errors later, then we need an unconditional step to check whether the preceding
expression reported one.
Discusssion: https://postgr.es/m/
---
src/backend/executor/execExpr.c | 31 ++++++-------------
src/backend/executor/execExprInterp.c | 7 +++++
.../regress/expected/sqljson_jsontable.out | 13 ++++++++
.../regress/expected/sqljson_queryfuncs.out | 16 ++++++++++
src/test/regress/sql/sqljson_jsontable.sql | 5 +++
src/test/regress/sql/sqljson_queryfuncs.sql | 7 +++++
6 files changed, 58 insertions(+), 21 deletions(-)
diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 82e846a1f4f..a9882bbb3c4 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -4951,16 +4951,11 @@ ExecInitJsonExpr(JsonExpr *jsexpr, ExprState *state,
* Add a COERCION_FINISH step to check for errors that may occur when
* coercing and rethrow them.
*/
- if (jsexpr->on_error->coerce ||
- IsA(jsexpr->on_error->expr, CoerceViaIO) ||
- IsA(jsexpr->on_error->expr, CoerceToDomain))
- {
- scratch->opcode = EEOP_JSONEXPR_COERCION_FINISH;
- scratch->resvalue = resv;
- scratch->resnull = resnull;
- scratch->d.jsonexpr.jsestate = jsestate;
- ExprEvalPushStep(state, scratch);
- }
+ scratch->opcode = EEOP_JSONEXPR_COERCION_FINISH;
+ scratch->resvalue = resv;
+ scratch->resnull = resnull;
+ scratch->d.jsonexpr.jsestate = jsestate;
+ ExprEvalPushStep(state, scratch);
/* JUMP to end to skip the ON EMPTY steps added below. */
jumps_to_end = lappend_int(jumps_to_end, state->steps_len);
@@ -5014,17 +5009,11 @@ ExecInitJsonExpr(JsonExpr *jsexpr, ExprState *state,
* Add a COERCION_FINISH step to check for errors that may occur when
* coercing and rethrow them.
*/
- if (jsexpr->on_empty->coerce ||
- IsA(jsexpr->on_empty->expr, CoerceViaIO) ||
- IsA(jsexpr->on_empty->expr, CoerceToDomain))
- {
-
- scratch->opcode = EEOP_JSONEXPR_COERCION_FINISH;
- scratch->resvalue = resv;
- scratch->resnull = resnull;
- scratch->d.jsonexpr.jsestate = jsestate;
- ExprEvalPushStep(state, scratch);
- }
+ scratch->opcode = EEOP_JSONEXPR_COERCION_FINISH;
+ scratch->resvalue = resv;
+ scratch->resnull = resnull;
+ scratch->d.jsonexpr.jsestate = jsestate;
+ ExprEvalPushStep(state, scratch);
}
foreach(lc, jumps_to_end)
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index 397219f7a3a..91d1043ecf6 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -5188,6 +5188,13 @@ ExecEvalJsonCoercion(ExprState *state, ExprEvalStep *op,
{
ErrorSaveContext *escontext = op->d.jsonexpr_coercion.escontext;
+ 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/test/regress/expected/sqljson_jsontable.out b/src/test/regress/expected/sqljson_jsontable.out
index ae64dbed303..d6a22ba8205 100644
--- a/src/test/regress/expected/sqljson_jsontable.out
+++ b/src/test/regress/expected/sqljson_jsontable.out
@@ -672,6 +672,19 @@ SELECT a, a::bool FROM JSON_TABLE(jsonb '{"a":1}', '$' COLUMNS (a dint4_0 EXISTS
1 | t
(1 row)
+-- soft errors in the ON EMPTY expression must be rethrown
+SELECT * FROM JSON_TABLE('{}', '$' COLUMNS (a int PATH '$.a' DEFAULT (0::dint4_0 + 0) ON EMPTY));
+ERROR: could not coerce ON EMPTY expression (DEFAULT) to the RETURNING type
+DETAIL: value for domain dint4_0 violates check constraint "dint4_0_check"
+SELECT * FROM JSON_TABLE('{}', '$' COLUMNS (a int PATH '$.a' KEEP QUOTES DEFAULT (0::dint4_0 + 0) ON EMPTY));
+ERROR: could not coerce ON EMPTY expression (DEFAULT) to the RETURNING type
+DETAIL: value for domain dint4_0 violates check constraint "dint4_0_check"
+SELECT * FROM JSON_TABLE('{}', '$' COLUMNS (a int PATH '$.a' DEFAULT (('a' || (random() * 0)::int)::int + 0) ON EMPTY));
+ERROR: could not coerce ON EMPTY expression (DEFAULT) to the RETURNING type
+DETAIL: invalid input syntax for type integer: "a0"
+SELECT * FROM JSON_TABLE('{}', '$' COLUMNS (a int PATH '$.a' KEEP QUOTES DEFAULT (('a' || (random() * 0)::int)::int + 0) ON EMPTY));
+ERROR: could not coerce ON EMPTY expression (DEFAULT) to the RETURNING type
+DETAIL: invalid input syntax for type integer: "a0"
DROP DOMAIN dint4, dint4_0;
-- JSON_TABLE: WRAPPER/QUOTES clauses on scalar columns
SELECT * FROM JSON_TABLE(jsonb '"world"', '$' COLUMNS (item text PATH '$' KEEP QUOTES ON SCALAR STRING));
diff --git a/src/test/regress/expected/sqljson_queryfuncs.out b/src/test/regress/expected/sqljson_queryfuncs.out
index ff64dce0c59..0c3672b67a2 100644
--- a/src/test/regress/expected/sqljson_queryfuncs.out
+++ b/src/test/regress/expected/sqljson_queryfuncs.out
@@ -1489,6 +1489,22 @@ ERROR: invalid ON ERROR behavior
LINE 1: SELECT JSON_QUERY(jsonb '1', '$' TRUE ON ERROR);
^
DETAIL: Only ERROR, NULL, EMPTY ARRAY, EMPTY OBJECT, or DEFAULT expression is allowed in ON ERROR for JSON_QUERY().
+-- DEFAULT expression ON ERROR / EMPTY soft error should rethrow unconditionally
+SELECT JSON_VALUE('"a"', '$' RETURNING sqljsonb_int_not_null DEFAULT NULL ON ERROR);
+ERROR: could not coerce ON ERROR expression (DEFAULT) to the RETURNING type
+DETAIL: domain sqljsonb_int_not_null does not allow null values
+SELECT JSON_VALUE('"a"', '$' RETURNING int DEFAULT (NULL::sqljsonb_int_not_null + 0) ON ERROR);
+ERROR: could not coerce ON ERROR expression (DEFAULT) to the RETURNING type
+DETAIL: domain sqljsonb_int_not_null does not allow null values
+SELECT JSON_QUERY('"a"', '$' RETURNING int DEFAULT (NULL::sqljsonb_int_not_null + 0) ON ERROR);
+ERROR: could not coerce ON ERROR expression (DEFAULT) to the RETURNING type
+DETAIL: domain sqljsonb_int_not_null does not allow null values
+SELECT JSON_VALUE('"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"
+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"
-- 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_jsontable.sql b/src/test/regress/sql/sqljson_jsontable.sql
index 2a33aaec57f..7f0b54b1121 100644
--- a/src/test/regress/sql/sqljson_jsontable.sql
+++ b/src/test/regress/sql/sqljson_jsontable.sql
@@ -311,6 +311,11 @@ SELECT a, a::bool FROM JSON_TABLE(jsonb '{"a":1}', '$' COLUMNS (a dint4_0 EXISTS
SELECT a, a::bool FROM JSON_TABLE(jsonb '{"a":1}', '$' COLUMNS (a dint4_0 EXISTS PATH '$.b' ERROR ON ERROR));
SELECT a, a::bool FROM JSON_TABLE(jsonb '{"a":1}', '$' COLUMNS (a dint4_0 EXISTS PATH '$.b' FALSE ON ERROR));
SELECT a, a::bool FROM JSON_TABLE(jsonb '{"a":1}', '$' COLUMNS (a dint4_0 EXISTS PATH '$.b' TRUE ON ERROR));
+-- soft errors in the ON EMPTY expression must be rethrown
+SELECT * FROM JSON_TABLE('{}', '$' COLUMNS (a int PATH '$.a' DEFAULT (0::dint4_0 + 0) ON EMPTY));
+SELECT * FROM JSON_TABLE('{}', '$' COLUMNS (a int PATH '$.a' KEEP QUOTES DEFAULT (0::dint4_0 + 0) ON EMPTY));
+SELECT * FROM JSON_TABLE('{}', '$' COLUMNS (a int PATH '$.a' DEFAULT (('a' || (random() * 0)::int)::int + 0) ON EMPTY));
+SELECT * FROM JSON_TABLE('{}', '$' COLUMNS (a int PATH '$.a' KEEP QUOTES DEFAULT (('a' || (random() * 0)::int)::int + 0) ON EMPTY));
DROP DOMAIN dint4, dint4_0;
-- JSON_TABLE: WRAPPER/QUOTES clauses on scalar columns
diff --git a/src/test/regress/sql/sqljson_queryfuncs.sql b/src/test/regress/sql/sqljson_queryfuncs.sql
index a69ef253f66..2f7e4b031be 100644
--- a/src/test/regress/sql/sqljson_queryfuncs.sql
+++ b/src/test/regress/sql/sqljson_queryfuncs.sql
@@ -493,6 +493,13 @@ SELECT JSON_EXISTS(jsonb '1', '$' DEFAULT 1 ON ERROR);
SELECT JSON_VALUE(jsonb '1', '$' EMPTY ON ERROR);
SELECT JSON_QUERY(jsonb '1', '$' TRUE ON ERROR);
+-- DEFAULT expression ON ERROR / EMPTY soft error should rethrow unconditionally
+SELECT JSON_VALUE('"a"', '$' RETURNING sqljsonb_int_not_null DEFAULT NULL ON ERROR);
+SELECT JSON_VALUE('"a"', '$' RETURNING int DEFAULT (NULL::sqljsonb_int_not_null + 0) ON ERROR);
+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);
+
-- Test implicit coercion to a domain over fixed-length type specified in
-- RETURNING
CREATE DOMAIN queryfuncs_char2 AS char(2);
--
2.34.1