public inbox for [email protected]
help / color / mirror / Atom feedFrom: Amit Langote <[email protected]>
To: Andrey Rachitskiy <[email protected]>
Cc: Andrey Borodin <[email protected]>
Cc: Nikita Malakhov <[email protected]>
Cc: PostgreSQL mailing lists <[email protected]>
Cc: Nikolay Shaplov <[email protected]>
Subject: Re: BUG #19458: OOM killer in jsonb_path_exists_opr (@?) with malformed JSONPath containing non-existent variables
Date: Wed, 17 Jun 2026 17:27:25 +0900
Message-ID: <CA+HiwqEL=Hr3ReVLqRy-U7JFOmx8ziS=_NLQcD4sA411kS5K9A@mail.gmail.com> (raw)
In-Reply-To: <CAB8bMivkf5X73X6LOSxt4r4tx_PN6DnGKVcBa3BtYX=eOKwHaw@mail.gmail.com>
References: <[email protected]>
<CAB8bMit1HvJsAasUYwmq+82Oa3zQhJyvsHNS4PGF_S_BCMnuVA@mail.gmail.com>
<[email protected]>
<CAN-LCVNh2z4EE+F21XPe7XRSWfPFtZx1WYAswpU5qs+RdR=jjg@mail.gmail.com>
<[email protected]>
<CAN-LCVPiBi9XXW__RorX=dH2_fANAMXhdbULmHFFg97F_0ubRw@mail.gmail.com>
<[email protected]>
<CA+HiwqG5pP8g0oGkz8x6X80XJyZqGiS16F9DhvJ3Ukejkd8MbQ@mail.gmail.com>
<CAB8bMivkf5X73X6LOSxt4r4tx_PN6DnGKVcBa3BtYX=eOKwHaw@mail.gmail.com>
Hi Andrey,
On Fri, Jun 5, 2026 at 7:03 PM Andrey Rachitskiy <[email protected]> wrote:
> The growing allocation is leaked temporary JsonValueLists in executePredicate() (local lseq/rseq, ~1482–1547) and the arithmetic helpers executeBinaryArithmExpr() / executeUnaryArithmExpr() (~1561–1684). Each nested comparison or arithmetic subexpression materializes operands via executeItemOptUnwrapResult[NoThrow]() → executeNextItem() → JsonValueListAppend() (~1165, ~2451), but the interim lists are never freed before return. For @? specifically, executeJsonPath() also leaks a local vals list in strict exists mode (~579–586).
>
> Missing vars make the AFL case worse by returning null instead of error, so evaluation continues deep into nested $?()/comparisons instead of stopping at the first $"…" reference. The same leak mechanism is reachable without missing vars — Tom Lane demonstrated this on master (5a2043bf713) with $[*] ? (@ < $) on a large array.
>
> Our missing-variable patch fixes the reported OOM and the @? semantics bug by aborting early. Whether REL_14/15/16 also need a broader fix for interim JsonValueList cleanup is beyond what I can confidently propose; I've tried to pin down where the growth happens for that discussion.
Thanks for that tracedown and for pointing to Tom's commit. The deeper
interim-JsonValueList leak looks unlikely to get fixed in the back
branches; Tom's cleanup (5a2043bf713) went only to master.
I'll look at committing the attached revised version of your Apr 20
patch (same fix, plus a regression test) down to REL_14. Please
check/test.
--
Thanks, Amit Langote
Attachments:
[application/octet-stream] v2-0001-Report-undefined-jsonpath-variable-when-no-variab.patch (4.7K, ../CA+HiwqEL=Hr3ReVLqRy-U7JFOmx8ziS=_NLQcD4sA411kS5K9A@mail.gmail.com/2-v2-0001-Report-undefined-jsonpath-variable-when-no-variab.patch)
download | inline diff:
From d52b3b5a728d5130e3e14acd778c5b608b9e50f3 Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Wed, 17 Jun 2026 17:21:38 +0900
Subject: [PATCH v2] Report undefined jsonpath variable when no variables are
supplied
The two-argument jsonb @? and @@ operators invoke the jsonpath executor
with no variable set. In that case getJsonPathVariable() treated any
"$name" reference as JSON null and continued evaluating, instead of
reporting the variable as undefined.
This produced incorrect results -- for example '42'::jsonb @? '$"x"'
returned true -- and, for some malformed or hostile jsonpath expressions
with deeply nested predicates, allowed essentially unbounded memory
consumption that could get the backend killed by the OOM killer.
Report the undefined variable as an error in this case as well, reusing
the message already emitted when a variable is not found among supplied
variables. This matches the behavior of v17 and later, where the
jsonpath executor was reorganized. Stopping at the first undefined
variable reference also resolves the reported memory-growth case.
Note this is a user-visible change in the back branches: a jsonpath
expression that references a variable while no variables are supplied now
raises an error rather than silently evaluating it as NULL. The previous
behavior was incorrect, so the change is judged worthwhile.
Author: Andrey Rachitskiy <[email protected]>
Reported-by: Andrey Rachitskiy <[email protected]>
Reviewed-by: Andrey Borodin <[email protected]>
Reviewed-by: Nikita Malakhov <[email protected]>
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 14
---
src/backend/utils/adt/jsonpath_exec.c | 13 +++++++------
src/test/regress/expected/jsonb_jsonpath.out | 7 +++++++
src/test/regress/sql/jsonb_jsonpath.sql | 5 +++++
3 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index 10ec66c6293..34420849eac 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -2128,14 +2128,15 @@ getJsonPathVariable(JsonPathExecContext *cxt, JsonPathItem *variable,
JsonbValue tmp;
JsonbValue *v;
- if (!vars)
- {
- value->type = jbvNull;
- return;
- }
-
Assert(variable->type == jpiVariable);
varName = jspGetString(variable, &varNameLength);
+
+ if (!vars)
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("could not find jsonpath variable \"%s\"",
+ pnstrdup(varName, varNameLength))));
+
tmp.type = jbvString;
tmp.val.string.val = varName;
tmp.val.string.len = varNameLength;
diff --git a/src/test/regress/expected/jsonb_jsonpath.out b/src/test/regress/expected/jsonb_jsonpath.out
index 6659bc9091a..2ab90643450 100644
--- a/src/test/regress/expected/jsonb_jsonpath.out
+++ b/src/test/regress/expected/jsonb_jsonpath.out
@@ -487,6 +487,13 @@ select * from jsonb_path_query('{"a": 10}', '$');
select * from jsonb_path_query('{"a": 10}', '$ ? (@.a < $value)');
ERROR: could not find jsonpath variable "value"
+-- bug #19458: the @? and @@ operators supply no variables, so a variable
+-- reference must be reported as undefined rather than silently treated as
+-- NULL (the latter gave wrong results and could drive unbounded memory use)
+select jsonb '42' @? '$"no_such_var"';
+ERROR: could not find jsonpath variable "no_such_var"
+select jsonb '42' @@ '$"no_such_var" == 1';
+ERROR: could not find jsonpath variable "no_such_var"
select * from jsonb_path_query('{"a": 10}', '$ ? (@.a < $value)', '1');
ERROR: "vars" argument is not an object
DETAIL: Jsonpath parameters should be encoded as key-value pairs of "vars" object.
diff --git a/src/test/regress/sql/jsonb_jsonpath.sql b/src/test/regress/sql/jsonb_jsonpath.sql
index e0ce509264a..f0e08d27e84 100644
--- a/src/test/regress/sql/jsonb_jsonpath.sql
+++ b/src/test/regress/sql/jsonb_jsonpath.sql
@@ -98,6 +98,11 @@ select jsonb_path_query('[1,2,3]', '$[last ? (@.type() == "string")]', silent =>
select * from jsonb_path_query('{"a": 10}', '$');
select * from jsonb_path_query('{"a": 10}', '$ ? (@.a < $value)');
+-- bug #19458: the @? and @@ operators supply no variables, so a variable
+-- reference must be reported as undefined rather than silently treated as
+-- NULL (the latter gave wrong results and could drive unbounded memory use)
+select jsonb '42' @? '$"no_such_var"';
+select jsonb '42' @@ '$"no_such_var" == 1';
select * from jsonb_path_query('{"a": 10}', '$ ? (@.a < $value)', '1');
select * from jsonb_path_query('{"a": 10}', '$ ? (@.a < $value)', '[{"value" : 13}]');
select * from jsonb_path_query('{"a": 10}', '$ ? (@.a < $value)', '{"value" : 13}');
--
2.47.3
view thread (13+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: BUG #19458: OOM killer in jsonb_path_exists_opr (@?) with malformed JSONPath containing non-existent variables
In-Reply-To: <CA+HiwqEL=Hr3ReVLqRy-U7JFOmx8ziS=_NLQcD4sA411kS5K9A@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox