agora inbox for pgsql-bugs@postgresql.org
help / color / mirror / Atom feedBUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY
6+ messages / 4 participants
[nested] [flat]
* BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY
@ 2026-08-15 03:25 PG Bug reporting form <noreply@postgresql.org>
2026-08-16 14:29 ` Re: BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY Andrey Rachitskiy <pl0h0yp1@gmail.com>
0 siblings, 1 reply; 6+ messages in thread
From: PG Bug reporting form @ 2026-08-15 03:25 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: syzhong16@gmail.com
The following bug has been logged on the website:
Bug reference: 19621
Logged by: Suyang Zhong
Email address: syzhong16@gmail.com
PostgreSQL version: 19beta3
Operating system: Ubuntu 22.04
Description:
Consider the following test case:
```sql
CREATE TABLE t0(x text);
INSERT INTO t0 VALUES ('{}'), (NULL);
CREATE VIEW v0 AS
SELECT x, json_value(x, '$.a' RETURNING int DEFAULT 42 ON EMPTY) AS jv
FROM t0;
SELECT x IS NULL AS is_null, jv FROM v0;
-- f | 42
-- t | 42
SELECT jv FROM v0 WHERE x IS NULL;
-- NULL
```
The second query selects exactly the row the first query shows as `t | 42`,
yet returns NULL for its `jv`.
A further reduction shows that the result depends on the input row order.
```sql
SELECT json_value(x, '$.a' RETURNING int DEFAULT 42 ON EMPTY) FROM (VALUES
('{}'), (NULL)) v(x);
-- 42
-- 42
SELECT json_value(x, '$.a' RETURNING int DEFAULT 42 ON EMPTY) FROM (VALUES
(NULL), ('{}')) v(x);
-- NULL
-- 42
```
Reproduced on 20devel, and on 17.11, 18.1, 18.4, and 19beta3; on 17rc1 only
the `ON ERROR` variants misbehave.
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY
2026-08-15 03:25 BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY PG Bug reporting form <noreply@postgresql.org>
@ 2026-08-16 14:29 ` Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-08-18 02:06 ` Re: BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY =?ISO-8859-1?B?emVuZ21hbg==?= <zengman@halodbtech.com>
0 siblings, 1 reply; 6+ messages in thread
From: Andrey Rachitskiy @ 2026-08-16 14:29 UTC (permalink / raw)
To: syzhong16@gmail.com; pgsql-bugs@lists.postgresql.org; +Cc: Amit Langote <amitlangote09@gmail.com>
вс, 16 авг. 2026 г. в 17:36, PG Bug reporting form <noreply@postgresql.org>:
> Consider the following test case:
>
> ```sql
> CREATE TABLE t0(x text);
> INSERT INTO t0 VALUES ('{}'), (NULL);
> CREATE VIEW v0 AS
> SELECT x, json_value(x, '$.a' RETURNING int DEFAULT 42 ON EMPTY) AS jv
> FROM t0;
>
> SELECT x IS NULL AS is_null, jv FROM v0;
> -- f | 42
> -- t | 42
>
> SELECT jv FROM v0 WHERE x IS NULL;
> -- NULL
> ```
>
> The second query selects exactly the row the first query shows as `t | 42`,
> yet returns NULL for its `jv`.
>
> A further reduction shows that the result depends on the input row order.
>
> ```sql
> SELECT json_value(x, '$.a' RETURNING int DEFAULT 42 ON EMPTY) FROM (VALUES
> ('{}'), (NULL)) v(x);
> -- 42
> -- 42
> SELECT json_value(x, '$.a' RETURNING int DEFAULT 42 ON EMPTY) FROM (VALUES
> (NULL), ('{}')) v(x);
> -- NULL
> -- 42
> ```
>
> Reproduced on 20devel, and on 17.11, 18.1, 18.4, and 19beta3; on 17rc1 only
> the `ON ERROR` variants misbehave.
>
> Hi, Suyang!
Thanks for the report!
On NULL input, jsonpath is deliberately not run: there is nothing to
search, so the result is NULL (a NOT NULL domain still checks the NULL).
The "found empty" / "had an error" flags were cleared only in the step
that runs jsonpath. NULL skips that step, so the previous row's flags
remain. The next check is "empty? then DEFAULT" — and it fires for the
wrong row.
This dates to SQL/JSON itself (6185c973, March 2024): NULL skips path
evaluation, the reset lived inside that evaluation. Later (dd8bea88abf)
the extra steps were omitted for the default "just return NULL", so the
bug stayed hidden. A non-NULL DEFAULT (42, TRUE ON ERROR) keeps those
steps — the bug shows. On 17rc1 the reporter mostly saw ON ERROR: ON
EMPTY DEFAULT did not always go through the same path yet.
Proposal fix
-----------
Clear "empty/error" at the start of each row, before deciding not to
run the path on NULL. NULL still does not run the path. The only change
is that another row's DEFAULT no longer sticks.
CC Amit Langote (JSON).
Attachments:
[text/x-patch] 0001-Reset-JsonExpr-empty-error-flags-before-NULL-short-circuit.patch (10.0K, ../../CAB8bMisC9N06fQbJ0ie02Qzv5mO4K8rA=_Gb0UFzMZLCwft4QQ@mail.gmail.com/3-0001-Reset-JsonExpr-empty-error-flags-before-NULL-short-circuit.patch)
download | inline diff:
From: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Date: Sun, 16 Aug 2026 19:24:00 +0500
Subject: [PATCH] Reset JsonExpr empty/error flags before NULL short-circuit
SQL NULL context/path skips EEOP_JSONEXPR_PATH (JUMP_IF_NULL to CONST NULL)
so jsonpath is not evaluated, then falls through to domain coercion and
ON EMPTY / ON ERROR checks. empty/error were cleared only inside PATH,
so a previous row's EMPTY/ERROR leaked onto the NULL row when DEFAULT was
not NULL.
Clear the flags in a new EEOP_JSONEXPR_RESET at the start of each JsonExpr
evaluation.
BUG #19621
Reported-by: Suyang Zhong <syzhong16@gmail.com>
Author: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Discussion: https://www.postgresql.org/message-id/19621-0a480d2dc74e6bd5%40postgresql.org
Backpatch-through: 17
---
diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index cfea7e160c2..a3050576cb3 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -4759,6 +4759,11 @@ ExecInitJsonExpr(JsonExpr *jsexpr, ExprState *state,
jsestate->jsexpr = jsexpr;
+ /* Clear empty/error here. SQL NULL skips PATH. */
+ scratch->opcode = EEOP_JSONEXPR_RESET;
+ scratch->d.jsonexpr.jsestate = jsestate;
+ ExprEvalPushStep(state, scratch);
+
/*
* Evaluate formatted_expr storing the result into
* jsestate->formatted_expr.
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index 9bc23cb16fa..a82f9ee11a5 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -578,6 +578,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
&&CASE_EEOP_XMLEXPR,
&&CASE_EEOP_JSON_CONSTRUCTOR,
&&CASE_EEOP_IS_JSON,
+ &&CASE_EEOP_JSONEXPR_RESET,
&&CASE_EEOP_JSONEXPR_PATH,
&&CASE_EEOP_JSONEXPR_COERCION,
&&CASE_EEOP_JSONEXPR_COERCION_FINISH,
@@ -1936,6 +1937,13 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
EEO_NEXT();
}
+ EEO_CASE(EEOP_JSONEXPR_RESET)
+ {
+ ExecEvalJsonExprReset(state, op);
+
+ EEO_NEXT();
+ }
+
EEO_CASE(EEOP_JSONEXPR_PATH)
{
/* too complex for an inline implementation */
@@ -4890,6 +4898,28 @@ ExecEvalJsonIsPredicate(ExprState *state, ExprEvalStep *op)
*op->resvalue = BoolGetDatum(res);
}
+/*
+ * ExecEvalJsonExprReset
+ * Clear empty/error and ErrorSaveContext for this evaluation.
+ *
+ * SQL NULL skips EEOP_JSONEXPR_PATH, so this cannot live there.
+ */
+void
+ExecEvalJsonExprReset(ExprState *state, ExprEvalStep *op)
+{
+ JsonExprState *jsestate = op->d.jsonexpr.jsestate;
+
+ memset(&jsestate->error, 0, sizeof(NullableDatum));
+ memset(&jsestate->empty, 0, sizeof(NullableDatum));
+
+ if (jsestate->escontext.details_wanted)
+ {
+ jsestate->escontext.error_data = NULL;
+ jsestate->escontext.details_wanted = false;
+ }
+ jsestate->escontext.error_occurred = false;
+}
+
/*
* Evaluate a jsonpath against a document, both of which must have been
* evaluated and their values saved in op->d.jsonexpr.jsestate.
@@ -4922,18 +4952,6 @@ ExecEvalJsonExprPath(ExprState *state, ExprEvalStep *op,
item = jsestate->formatted_expr.value;
path = DatumGetJsonPathP(jsestate->pathspec.value);
- /* Set error/empty to false. */
- memset(&jsestate->error, 0, sizeof(NullableDatum));
- memset(&jsestate->empty, 0, sizeof(NullableDatum));
-
- /* Also reset ErrorSaveContext contents for the next row. */
- if (jsestate->escontext.details_wanted)
- {
- jsestate->escontext.error_data = NULL;
- jsestate->escontext.details_wanted = false;
- }
- jsestate->escontext.error_occurred = false;
-
switch (jsexpr->op)
{
case JSON_EXISTS_OP:
diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c
index 29617437477..a96c09afe23 100644
--- a/src/backend/jit/llvm/llvmjit_expr.c
+++ b/src/backend/jit/llvm/llvmjit_expr.c
@@ -2259,6 +2259,12 @@ llvm_compile_expr(ExprState *state)
LLVMBuildBr(b, opblocks[opno + 1]);
break;
+ case EEOP_JSONEXPR_RESET:
+ build_EvalXFunc(b, mod, "ExecEvalJsonExprReset",
+ v_state, op);
+ LLVMBuildBr(b, opblocks[opno + 1]);
+ break;
+
case EEOP_JSONEXPR_PATH:
{
JsonExprState *jsestate = op->d.jsonexpr.jsestate;
diff --git a/src/backend/jit/llvm/llvmjit_types.c b/src/backend/jit/llvm/llvmjit_types.c
index c8a1f841293..654600da5b0 100644
--- a/src/backend/jit/llvm/llvmjit_types.c
+++ b/src/backend/jit/llvm/llvmjit_types.c
@@ -173,6 +173,7 @@ void *referenced_functions[] =
ExecEvalXmlExpr,
ExecEvalJsonConstructor,
ExecEvalJsonIsPredicate,
+ ExecEvalJsonExprReset,
ExecEvalJsonCoercion,
ExecEvalJsonCoercionFinish,
ExecEvalJsonExprPath,
diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h
index c61b3d624d5..8afc09d5dfa 100644
--- a/src/include/executor/execExpr.h
+++ b/src/include/executor/execExpr.h
@@ -265,6 +265,7 @@ typedef enum ExprEvalOp
EEOP_XMLEXPR,
EEOP_JSON_CONSTRUCTOR,
EEOP_IS_JSON,
+ EEOP_JSONEXPR_RESET,
EEOP_JSONEXPR_PATH,
EEOP_JSONEXPR_COERCION,
EEOP_JSONEXPR_COERCION_FINISH,
@@ -754,7 +755,7 @@ typedef struct ExprEvalStep
JsonIsPredicate *pred; /* original expression node */
} is_json;
- /* for EEOP_JSONEXPR_PATH */
+ /* for EEOP_JSONEXPR_RESET, EEOP_JSONEXPR_PATH, EEOP_JSONEXPR_COERCION_FINISH */
struct
{
struct JsonExprState *jsestate;
@@ -892,6 +893,7 @@ extern void ExecEvalXmlExpr(ExprState *state, ExprEvalStep *op);
extern void ExecEvalJsonConstructor(ExprState *state, ExprEvalStep *op,
ExprContext *econtext);
extern void ExecEvalJsonIsPredicate(ExprState *state, ExprEvalStep *op);
+extern void ExecEvalJsonExprReset(ExprState *state, ExprEvalStep *op);
extern int ExecEvalJsonExprPath(ExprState *state, ExprEvalStep *op,
ExprContext *econtext);
extern void ExecEvalJsonCoercion(ExprState *state, ExprEvalStep *op,
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index e95ac3eda35..fc8c6e21ad2 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1099,7 +1099,7 @@ typedef struct DomainConstraintState
* State for JsonExpr evaluation, too big to inline.
*
* This contains the information going into and coming out of the
- * EEOP_JSONEXPR_PATH eval step.
+ * EEOP_JSONEXPR_RESET / EEOP_JSONEXPR_PATH eval steps.
*/
typedef struct JsonExprState
{
@@ -1119,7 +1119,7 @@ typedef struct JsonExprState
* Output variables that drive the EEOP_JUMP_IF_NOT_TRUE steps that are
* added for ON ERROR and ON EMPTY expressions, if any.
*
- * Reset for each evaluation of EEOP_JSONEXPR_PATH.
+ * Cleared by EEOP_JSONEXPR_RESET at the start of each evaluation.
*/
/* Set to true if jsonpath evaluation cause an error. */
@@ -1161,7 +1161,7 @@ typedef struct JsonExprState
* not ERROR, a pointer to this is passed to ExecInitExprRec() when
* initializing the coercion expressions or to ExecInitJsonCoercion().
*
- * Reset for each evaluation of EEOP_JSONEXPR_PATH.
+ * Reset by EEOP_JSONEXPR_RESET at the start of each evaluation.
*/
ErrorSaveContext escontext;
} JsonExprState;
diff --git a/src/test/regress/expected/sqljson_queryfuncs.out b/src/test/regress/expected/sqljson_queryfuncs.out
index ff64dce0c59..b5209bae5e7 100644
--- a/src/test/regress/expected/sqljson_queryfuncs.out
+++ b/src/test/regress/expected/sqljson_queryfuncs.out
@@ -475,6 +475,57 @@ FROM
2 | -1
(3 rows)
+-- SQL NULL must not reuse a previous row's ON EMPTY / ON ERROR result.
+SELECT x IS NULL AS is_null,
+ json_value(x, '$.a' RETURNING int DEFAULT 42 ON EMPTY) AS jv
+FROM (VALUES ('{}'), (NULL), ('{}')) v(x);
+ is_null | jv
+---------+----
+ f | 42
+ t |
+ f | 42
+(3 rows)
+
+SELECT x IS NULL AS is_null,
+ json_value(x, 'strict $.a' RETURNING int DEFAULT 42 ON ERROR) AS jv
+FROM (VALUES ('1'), (NULL), ('1')) v(x);
+ is_null | jv
+---------+----
+ f | 42
+ t |
+ f | 42
+(3 rows)
+
+SELECT p IS NULL AS is_null,
+ json_value('{}', p RETURNING int DEFAULT 42 ON EMPTY) AS jv
+FROM (VALUES ('$.a'::jsonpath), (NULL), ('$.a'::jsonpath)) v(p);
+ is_null | jv
+---------+----
+ f | 42
+ t |
+ f | 42
+(3 rows)
+
+SELECT x IS NULL AS is_null,
+ json_query(x, '$.a' DEFAULT '"empty"' ON EMPTY) AS jq
+FROM (VALUES ('{}'::jsonb), (NULL), ('{}'::jsonb)) v(x);
+ is_null | jq
+---------+---------
+ f | "empty"
+ t |
+ f | "empty"
+(3 rows)
+
+SELECT x IS NULL AS is_null,
+ json_exists(x, 'strict $.a' TRUE ON ERROR) AS je
+FROM (VALUES ('1'::jsonb), (NULL), ('1'::jsonb)) v(x);
+ is_null | je
+---------+----
+ f | t
+ t |
+ f | t
+(3 rows)
+
SELECT JSON_VALUE(jsonb 'null', '$a' PASSING point ' (1, 2 )' AS a);
json_value
------------
diff --git a/src/test/regress/sql/sqljson_queryfuncs.sql b/src/test/regress/sql/sqljson_queryfuncs.sql
index a69ef253f66..03f6a932b9c 100644
--- a/src/test/regress/sql/sqljson_queryfuncs.sql
+++ b/src/test/regress/sql/sqljson_queryfuncs.sql
@@ -128,6 +128,27 @@ SELECT
FROM
generate_series(0, 2) x;
+-- SQL NULL must not reuse a previous row's ON EMPTY / ON ERROR result.
+SELECT x IS NULL AS is_null,
+ json_value(x, '$.a' RETURNING int DEFAULT 42 ON EMPTY) AS jv
+FROM (VALUES ('{}'), (NULL), ('{}')) v(x);
+
+SELECT x IS NULL AS is_null,
+ json_value(x, 'strict $.a' RETURNING int DEFAULT 42 ON ERROR) AS jv
+FROM (VALUES ('1'), (NULL), ('1')) v(x);
+
+SELECT p IS NULL AS is_null,
+ json_value('{}', p RETURNING int DEFAULT 42 ON EMPTY) AS jv
+FROM (VALUES ('$.a'::jsonpath), (NULL), ('$.a'::jsonpath)) v(p);
+
+SELECT x IS NULL AS is_null,
+ json_query(x, '$.a' DEFAULT '"empty"' ON EMPTY) AS jq
+FROM (VALUES ('{}'::jsonb), (NULL), ('{}'::jsonb)) v(x);
+
+SELECT x IS NULL AS is_null,
+ json_exists(x, 'strict $.a' TRUE ON ERROR) AS je
+FROM (VALUES ('1'::jsonb), (NULL), ('1'::jsonb)) v(x);
+
SELECT JSON_VALUE(jsonb 'null', '$a' PASSING point ' (1, 2 )' AS a);
SELECT JSON_VALUE(jsonb 'null', '$a' PASSING point ' (1, 2 )' AS a RETURNING point);
SELECT JSON_VALUE(jsonb 'null', '$a' PASSING point ' (1, 2 )' AS a RETURNING point ERROR ON ERROR);
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY
2026-08-15 03:25 BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY PG Bug reporting form <noreply@postgresql.org>
2026-08-16 14:29 ` Re: BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY Andrey Rachitskiy <pl0h0yp1@gmail.com>
@ 2026-08-18 02:06 ` =?ISO-8859-1?B?emVuZ21hbg==?= <zengman@halodbtech.com>
2026-08-18 08:56 ` Re: BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY Andrey Rachitskiy <pl0h0yp1@gmail.com>
0 siblings, 1 reply; 6+ messages in thread
From: zengman @ 2026-08-18 02:06 UTC (permalink / raw)
To: Andrey Rachitskiy <pl0h0yp1@gmail.com>; syzhong16 <syzhong16@gmail.com>; pgsql-bugs <pgsql-bugs@lists.postgresql.org>; +Cc: Amit Langote <amitlangote09@gmail.com>
Hi everyone,
I have another question related to JSON functions. Although it is a different issue, I wonder whether it would make sense to handle both cases in the same patch.
I tested the current patch, but it does not seem to address the problem I reported here:
```
https://www.postgresql.org/message-id/19625-683b498c92087bc8%40postgresql.org
```
--
Regards,
Man Zeng
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY
2026-08-15 03:25 BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY PG Bug reporting form <noreply@postgresql.org>
2026-08-16 14:29 ` Re: BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-08-18 02:06 ` Re: BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY =?ISO-8859-1?B?emVuZ21hbg==?= <zengman@halodbtech.com>
@ 2026-08-18 08:56 ` Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-09-04 10:38 ` Re: BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY Andrey Rachitskiy <pl0h0yp1@gmail.com>
0 siblings, 1 reply; 6+ messages in thread
From: Andrey Rachitskiy @ 2026-08-18 08:56 UTC (permalink / raw)
To: zengman <zengman@halodbtech.com>; +Cc: syzhong16 <syzhong16@gmail.com>; pgsql-bugs <pgsql-bugs@lists.postgresql.org>; Amit Langote <amitlangote09@gmail.com>
вт, 18 авг. 2026 г. в 07:06, zengman <zengman@halodbtech.com>:
> I tested the current patch, but it does not seem to address the problem I
> reported here:
> ```
>
> https://www.postgresql.org/message-id/19625-683b498c92087bc8%40postgresql.org
> ```
>
Dear Zeng,
One is per-row executor state. The other is a wrong parse-time rewrite of a
boolean DEFAULT. They share only that both involve SQL/JSON DEFAULT.
Combining them would mix an executor opcode change with a parser coercion
change, and it would make review and back-patching harder.
--
Regards,
Rachitskiy Andrey
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY
2026-08-15 03:25 BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY PG Bug reporting form <noreply@postgresql.org>
2026-08-16 14:29 ` Re: BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-08-18 02:06 ` Re: BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY =?ISO-8859-1?B?emVuZ21hbg==?= <zengman@halodbtech.com>
2026-08-18 08:56 ` Re: BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY Andrey Rachitskiy <pl0h0yp1@gmail.com>
@ 2026-09-04 10:38 ` Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-09-23 20:12 ` Re: BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY Manu <manuelreyesbravo@gmail.com>
0 siblings, 1 reply; 6+ messages in thread
From: Andrey Rachitskiy @ 2026-09-04 10:38 UTC (permalink / raw)
To: zengman <zengman@halodbtech.com>; +Cc: syzhong16 <syzhong16@gmail.com>; pgsql-bugs <pgsql-bugs@lists.postgresql.org>; Amit Langote <amitlangote09@gmail.com>
вт, 18 авг. 2026 г. в 13:56, Andrey Rachitskiy <pl0h0yp1@gmail.com>:
>
> вт, 18 авг. 2026 г. в 07:06, zengman <zengman@halodbtech.com>:
>
>> I tested the current patch, but it does not seem to address the problem I
>> reported here:
>> ```
>>
>> https://www.postgresql.org/message-id/19625-683b498c92087bc8%40postgresql.org
>> ```
>>
> Dear Zeng,
>
> One is per-row executor state. The other is a wrong parse-time rewrite of
> a boolean DEFAULT. They share only that both involve SQL/JSON DEFAULT.
> Combining them would mix an executor opcode change with a parser coercion
> change, and it would make review and back-patching harder.
>
>
Dear Amit,
Attached is v2 of the patch.
v1 was a malformed unified diff: three context lines after the
ExecEvalJsonIsPredicate hunk were missing the leading space, so
git apply rejected the file as corrupt. There is no code change
versus v1.
I also re-checked the reporter's JSON_EXISTS / JSON_VALUE /
JSON_QUERY examples from the later duplicate report [0]; they pass
with this patch.
[0]
https://www.postgresql.org/message-id/19654-3acd06154d027634@postgresql.org
--
Regards,
Rachitskiy Andrey
Attachments:
[text/x-patch] v2-0001-Reset-JsonExpr-empty-error-flags-before-NULL-short-circuit.patch (10.2K, ../../CAB8bMitawD=ERVLwYuDXvMT_OJqf+qAKx=3GEH6V_WHT7jnq7A@mail.gmail.com/3-v2-0001-Reset-JsonExpr-empty-error-flags-before-NULL-short-circuit.patch)
download | inline diff:
From: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Date: Sun, 16 Aug 2026 19:24:00 +0500
Subject: [PATCH v2] Reset JsonExpr empty/error flags before NULL short-circuit
SQL NULL context/path skips EEOP_JSONEXPR_PATH (JUMP_IF_NULL to CONST NULL)
so jsonpath is not evaluated, then falls through to domain coercion and
ON EMPTY / ON ERROR checks. empty/error were cleared only inside PATH,
so a previous row's EMPTY/ERROR leaked onto the NULL row when DEFAULT was
not NULL.
Clear the flags in a new EEOP_JSONEXPR_RESET at the start of each JsonExpr
evaluation.
BUG #19621
Reported-by: Suyang Zhong <syzhong16@gmail.com>
Author: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Discussion: https://www.postgresql.org/message-id/19621-0a480d2dc74e6bd5%40postgresql.org
Backpatch-through: 17
---
diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index cfea7e160c2..a3050576cb3 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -4759,6 +4759,11 @@ ExecInitJsonExpr(JsonExpr *jsexpr, ExprState *state,
jsestate->jsexpr = jsexpr;
+ /* Clear empty/error here. SQL NULL skips PATH. */
+ scratch->opcode = EEOP_JSONEXPR_RESET;
+ scratch->d.jsonexpr.jsestate = jsestate;
+ ExprEvalPushStep(state, scratch);
+
/*
* Evaluate formatted_expr storing the result into
* jsestate->formatted_expr.
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index 9bc23cb16fa..a82f9ee11a5 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -578,6 +578,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
&&CASE_EEOP_XMLEXPR,
&&CASE_EEOP_JSON_CONSTRUCTOR,
&&CASE_EEOP_IS_JSON,
+ &&CASE_EEOP_JSONEXPR_RESET,
&&CASE_EEOP_JSONEXPR_PATH,
&&CASE_EEOP_JSONEXPR_COERCION,
&&CASE_EEOP_JSONEXPR_COERCION_FINISH,
@@ -1936,6 +1937,13 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
EEO_NEXT();
}
+ EEO_CASE(EEOP_JSONEXPR_RESET)
+ {
+ ExecEvalJsonExprReset(state, op);
+
+ EEO_NEXT();
+ }
+
EEO_CASE(EEOP_JSONEXPR_PATH)
{
/* too complex for an inline implementation */
@@ -4890,8 +4898,34 @@ ExecEvalJsonIsPredicate(ExprState *state, ExprEvalStep *op)
*op->resvalue = BoolGetDatum(res);
}
+/*
+ * ExecEvalJsonExprReset
+ * Clear empty/error and ErrorSaveContext for this evaluation.
+ *
+ * SQL NULL skips EEOP_JSONEXPR_PATH, so this cannot live there.
+ */
+void
+ExecEvalJsonExprReset(ExprState *state, ExprEvalStep *op)
+{
+ JsonExprState *jsestate = op->d.jsonexpr.jsestate;
+
+ memset(&jsestate->error, 0, sizeof(NullableDatum));
+ memset(&jsestate->empty, 0, sizeof(NullableDatum));
+
+ if (jsestate->escontext.details_wanted)
+ {
+ jsestate->escontext.error_data = NULL;
+ jsestate->escontext.details_wanted = false;
+ }
+ jsestate->escontext.error_occurred = false;
+}
+
/*
* Evaluate a jsonpath against a document, both of which must have been
* evaluated and their values saved in op->d.jsonexpr.jsestate.
*
+ * JsonExprState.empty/error and ErrorSaveContext are already cleared by
+ * EEOP_JSONEXPR_RESET. This step only sets them. SQL NULL skips this
+ * step, so they must not be cleared here.
+ *
* If an error occurs during JsonPath* evaluation or when coercing its result
@@ -4922,18 +4956,6 @@ ExecEvalJsonExprPath(ExprState *state, ExprEvalStep *op,
item = jsestate->formatted_expr.value;
path = DatumGetJsonPathP(jsestate->pathspec.value);
- /* Set error/empty to false. */
- memset(&jsestate->error, 0, sizeof(NullableDatum));
- memset(&jsestate->empty, 0, sizeof(NullableDatum));
-
- /* Also reset ErrorSaveContext contents for the next row. */
- if (jsestate->escontext.details_wanted)
- {
- jsestate->escontext.error_data = NULL;
- jsestate->escontext.details_wanted = false;
- }
- jsestate->escontext.error_occurred = false;
-
switch (jsexpr->op)
{
case JSON_EXISTS_OP:
diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c
index 29617437477..a96c09afe23 100644
--- a/src/backend/jit/llvm/llvmjit_expr.c
+++ b/src/backend/jit/llvm/llvmjit_expr.c
@@ -2259,6 +2259,12 @@ llvm_compile_expr(ExprState *state)
LLVMBuildBr(b, opblocks[opno + 1]);
break;
+ case EEOP_JSONEXPR_RESET:
+ build_EvalXFunc(b, mod, "ExecEvalJsonExprReset",
+ v_state, op);
+ LLVMBuildBr(b, opblocks[opno + 1]);
+ break;
+
case EEOP_JSONEXPR_PATH:
{
JsonExprState *jsestate = op->d.jsonexpr.jsestate;
diff --git a/src/backend/jit/llvm/llvmjit_types.c b/src/backend/jit/llvm/llvmjit_types.c
index c8a1f841293..654600da5b0 100644
--- a/src/backend/jit/llvm/llvmjit_types.c
+++ b/src/backend/jit/llvm/llvmjit_types.c
@@ -173,6 +173,7 @@ void *referenced_functions[] =
ExecEvalXmlExpr,
ExecEvalJsonConstructor,
ExecEvalJsonIsPredicate,
+ ExecEvalJsonExprReset,
ExecEvalJsonCoercion,
ExecEvalJsonCoercionFinish,
ExecEvalJsonExprPath,
diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h
index c61b3d624d5..8afc09d5dfa 100644
--- a/src/include/executor/execExpr.h
+++ b/src/include/executor/execExpr.h
@@ -265,6 +265,7 @@ typedef enum ExprEvalOp
EEOP_XMLEXPR,
EEOP_JSON_CONSTRUCTOR,
EEOP_IS_JSON,
+ EEOP_JSONEXPR_RESET,
EEOP_JSONEXPR_PATH,
EEOP_JSONEXPR_COERCION,
EEOP_JSONEXPR_COERCION_FINISH,
@@ -754,7 +755,7 @@ typedef struct ExprEvalStep
JsonIsPredicate *pred; /* original expression node */
} is_json;
- /* for EEOP_JSONEXPR_PATH */
+ /* for EEOP_JSONEXPR_RESET, EEOP_JSONEXPR_PATH, EEOP_JSONEXPR_COERCION_FINISH */
struct
{
struct JsonExprState *jsestate;
@@ -892,6 +893,7 @@ extern void ExecEvalXmlExpr(ExprState *state, ExprEvalStep *op);
extern void ExecEvalJsonConstructor(ExprState *state, ExprEvalStep *op,
ExprContext *econtext);
extern void ExecEvalJsonIsPredicate(ExprState *state, ExprEvalStep *op);
+extern void ExecEvalJsonExprReset(ExprState *state, ExprEvalStep *op);
extern int ExecEvalJsonExprPath(ExprState *state, ExprEvalStep *op,
ExprContext *econtext);
extern void ExecEvalJsonCoercion(ExprState *state, ExprEvalStep *op,
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index e95ac3eda35..fc8c6e21ad2 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1099,7 +1099,7 @@ typedef struct DomainConstraintState
* State for JsonExpr evaluation, too big to inline.
*
* This contains the information going into and coming out of the
- * EEOP_JSONEXPR_PATH eval step.
+ * EEOP_JSONEXPR_RESET / EEOP_JSONEXPR_PATH eval steps.
*/
typedef struct JsonExprState
{
@@ -1119,7 +1119,7 @@ typedef struct JsonExprState
* Output variables that drive the EEOP_JUMP_IF_NOT_TRUE steps that are
* added for ON ERROR and ON EMPTY expressions, if any.
*
- * Reset for each evaluation of EEOP_JSONEXPR_PATH.
+ * Cleared by EEOP_JSONEXPR_RESET at the start of each evaluation.
*/
/* Set to true if jsonpath evaluation cause an error. */
@@ -1161,7 +1161,7 @@ typedef struct JsonExprState
* not ERROR, a pointer to this is passed to ExecInitExprRec() when
* initializing the coercion expressions or to ExecInitJsonCoercion().
*
- * Reset for each evaluation of EEOP_JSONEXPR_PATH.
+ * Reset by EEOP_JSONEXPR_RESET at the start of each evaluation.
*/
ErrorSaveContext escontext;
} JsonExprState;
diff --git a/src/test/regress/expected/sqljson_queryfuncs.out b/src/test/regress/expected/sqljson_queryfuncs.out
index ff64dce0c59..b5209bae5e7 100644
--- a/src/test/regress/expected/sqljson_queryfuncs.out
+++ b/src/test/regress/expected/sqljson_queryfuncs.out
@@ -475,6 +475,57 @@ FROM
2 | -1
(3 rows)
+-- SQL NULL must not reuse a previous row's ON EMPTY / ON ERROR result.
+SELECT x IS NULL AS is_null,
+ json_value(x, '$.a' RETURNING int DEFAULT 42 ON EMPTY) AS jv
+FROM (VALUES ('{}'), (NULL), ('{}')) v(x);
+ is_null | jv
+---------+----
+ f | 42
+ t |
+ f | 42
+(3 rows)
+
+SELECT x IS NULL AS is_null,
+ json_value(x, 'strict $.a' RETURNING int DEFAULT 42 ON ERROR) AS jv
+FROM (VALUES ('1'), (NULL), ('1')) v(x);
+ is_null | jv
+---------+----
+ f | 42
+ t |
+ f | 42
+(3 rows)
+
+SELECT p IS NULL AS is_null,
+ json_value('{}', p RETURNING int DEFAULT 42 ON EMPTY) AS jv
+FROM (VALUES ('$.a'::jsonpath), (NULL), ('$.a'::jsonpath)) v(p);
+ is_null | jv
+---------+----
+ f | 42
+ t |
+ f | 42
+(3 rows)
+
+SELECT x IS NULL AS is_null,
+ json_query(x, '$.a' DEFAULT '"empty"' ON EMPTY) AS jq
+FROM (VALUES ('{}'::jsonb), (NULL), ('{}'::jsonb)) v(x);
+ is_null | jq
+---------+---------
+ f | "empty"
+ t |
+ f | "empty"
+(3 rows)
+
+SELECT x IS NULL AS is_null,
+ json_exists(x, 'strict $.a' TRUE ON ERROR) AS je
+FROM (VALUES ('1'::jsonb), (NULL), ('1'::jsonb)) v(x);
+ is_null | je
+---------+----
+ f | t
+ t |
+ f | t
+(3 rows)
+
SELECT JSON_VALUE(jsonb 'null', '$a' PASSING point ' (1, 2 )' AS a);
json_value
------------
diff --git a/src/test/regress/sql/sqljson_queryfuncs.sql b/src/test/regress/sql/sqljson_queryfuncs.sql
index a69ef253f66..03f6a932b9c 100644
--- a/src/test/regress/sql/sqljson_queryfuncs.sql
+++ b/src/test/regress/sql/sqljson_queryfuncs.sql
@@ -128,6 +128,27 @@ SELECT
FROM
generate_series(0, 2) x;
+-- SQL NULL must not reuse a previous row's ON EMPTY / ON ERROR result.
+SELECT x IS NULL AS is_null,
+ json_value(x, '$.a' RETURNING int DEFAULT 42 ON EMPTY) AS jv
+FROM (VALUES ('{}'), (NULL), ('{}')) v(x);
+
+SELECT x IS NULL AS is_null,
+ json_value(x, 'strict $.a' RETURNING int DEFAULT 42 ON ERROR) AS jv
+FROM (VALUES ('1'), (NULL), ('1')) v(x);
+
+SELECT p IS NULL AS is_null,
+ json_value('{}', p RETURNING int DEFAULT 42 ON EMPTY) AS jv
+FROM (VALUES ('$.a'::jsonpath), (NULL), ('$.a'::jsonpath)) v(p);
+
+SELECT x IS NULL AS is_null,
+ json_query(x, '$.a' DEFAULT '"empty"' ON EMPTY) AS jq
+FROM (VALUES ('{}'::jsonb), (NULL), ('{}'::jsonb)) v(x);
+
+SELECT x IS NULL AS is_null,
+ json_exists(x, 'strict $.a' TRUE ON ERROR) AS je
+FROM (VALUES ('1'::jsonb), (NULL), ('1'::jsonb)) v(x);
+
SELECT JSON_VALUE(jsonb 'null', '$a' PASSING point ' (1, 2 )' AS a);
SELECT JSON_VALUE(jsonb 'null', '$a' PASSING point ' (1, 2 )' AS a RETURNING point);
SELECT JSON_VALUE(jsonb 'null', '$a' PASSING point ' (1, 2 )' AS a RETURNING point ERROR ON ERROR);
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY
2026-08-15 03:25 BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY PG Bug reporting form <noreply@postgresql.org>
2026-08-16 14:29 ` Re: BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-08-18 02:06 ` Re: BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY =?ISO-8859-1?B?emVuZ21hbg==?= <zengman@halodbtech.com>
2026-08-18 08:56 ` Re: BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-09-04 10:38 ` Re: BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY Andrey Rachitskiy <pl0h0yp1@gmail.com>
@ 2026-09-23 20:12 ` Manu <manuelreyesbravo@gmail.com>
0 siblings, 0 replies; 6+ messages in thread
From: Manu @ 2026-09-23 20:12 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: Andrey Rachitskiy <pl0h0yp1@gmail.com>; Amit Langote <amitlangote09@gmail.com>; syzhong16@gmail.com
Hi Andrey,
I tested v2 on master, REL_18_STABLE and REL_17_STABLE, together with
Srinath's fix for #19695 [1], which touches the same function. Both
apply cleanly on all three branches, build without warnings, and the
main regression suite passes. The reported cases now give the right
answer on the three branches:
- #19621: json_value(x, '$.a' RETURNING int DEFAULT 42 ON EMPTY) over
'{}' then NULL returns 42, NULL (it was 42, 42)
- #19654: json_exists(j, 'strict $.a' FALSE ON ERROR) over '{}' then
NULL returns false, NULL (it was false, false)
I could not exercise the llvmjit_expr.c part: this machine has no LLVM
build.
One thing for the back-patch. EEOP_JSONEXPR_RESET is inserted before
EEOP_JSONEXPR_PATH, so every opcode from there to EEOP_LAST changes its
value: 24 of them on REL_17_STABLE and 25 on REL_18_STABLE. On master
that is fine, but the back branches are under the buildfarm's ABI
compliance check (.abi-compliance-history), and anything built against
a minor release that uses those values would be off by one after the
update. Putting the new opcode just before EEOP_LAST in the
back-branch versions, with the dispatch table and llvmjit_expr.c
following the same order, would leave the existing values alone.
If you post back-branch versions, I will rerun the same checks on
them.
[1]
https://postgr.es/m/CAFC+b6p8Oo3_pLXPa=DVXx1XDQJ09b2W3U3WKPtkQYyh0z0z-A@mail.gmail.com
Regards,
Manu
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2026-09-23 20:12 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-08-15 03:25 BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY PG Bug reporting form <noreply@postgresql.org>
2026-08-16 14:29 ` Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-08-18 02:06 ` =?ISO-8859-1?B?emVuZ21hbg==?= <zengman@halodbtech.com>
2026-08-18 08:56 ` Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-09-04 10:38 ` Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-09-23 20:12 ` Manu <manuelreyesbravo@gmail.com>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox