"David E. Wheeler" <[email protected]> writes:
> On Jan 20, 2024, at 12:34, Tom Lane <[email protected]> wrote:
>> Surely we're not helping anybody by leaving that behavior in place.
>> Making it do something useful, throwing an error, or returning NULL
>> all seem superior to this. I observe that @@ returns NULL for the
>> path type it doesn't like, so maybe that's what to do here.
> I agree it would be far better for the behavior to be consistent, but frankly
> would like to see them raise an error. Ideally the hit would suggest the
> proper alternative operator or function to use, and maybe link to the docs
> that describe the difference between SQL-standard JSONPath and "predicate
> check expressions”, and how they have separate operators and functions.
That ship's probably sailed. However, I spent some time poking into
the odd behavior I showed for @?, and it seems to me that it's an
oversight in appendBoolResult. That just automatically returns jperOk
in the !found short-circuit path for any boolean result, which is not
the behavior you'd get if the boolean value were actually returned
(cf. jsonb_path_match_internal). I experimented with making it do
what seems like the right thing, and found that there is only one
regression test case that changes behavior:
select jsonb '2' @? '$ == "2"';
?column?
----------
- t
+ f
(1 row)
Now, JSON does not think that numeric 2 equals string "2", so
ISTM the expected output here is flat wrong. It's certainly
inconsistent with @@:
regression=# select jsonb '2' @@ '$ == "2"';
?column?
----------
(1 row)
So I think we should consider a patch like the attached
(probably with some more test cases added). I don't really
understand this code however, so maybe I missed something.
regards, tom lane
diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index ac16f5c85d..63c46cfab5 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -2065,7 +2065,14 @@ appendBoolResult(JsonPathExecContext *cxt, JsonPathItem *jsp,
JsonbValue jbv;
if (!jspGetNext(jsp, &next) && !found)
- return jperOk; /* found singleton boolean value */
+ {
+ /*
+ * We have a predicate check expression, i.e. a path ending in a bare
+ * boolean operator, and we don't need to return the exact value(s)
+ * found. Just report success or failure of the boolean.
+ */
+ return (res == jpbTrue) ? jperOk : jperNotFound;
+ }
if (res == jpbUnknown)
{
diff --git a/src/test/regress/expected/jsonb_jsonpath.out b/src/test/regress/expected/jsonb_jsonpath.out
index 6659bc9091..59e1b71051 100644
--- a/src/test/regress/expected/jsonb_jsonpath.out
+++ b/src/test/regress/expected/jsonb_jsonpath.out
@@ -1157,7 +1157,7 @@ select jsonb_path_query('2', '$ == "2"');
select jsonb '2' @? '$ == "2"';
?column?
----------
- t
+ f
(1 row)
select jsonb '2' @@ '$ > 1';