Patch bug: Fix jsonpath .* on Arrays
Hackers,
The behavior of the .* jpiAnyKey jsonpath selector seems incorrect.
```
select jsonb_path_query('[1,2,3]', '$.*');
jsonb_path_query
------------------
(0 rows)
select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', '$.*');
jsonb_path_query
------------------
[3, 4, 5]
```
The first example might be expected, since .* is intended for object keys, but the handing of `jpiAnyKey` has a branch for unwrapping arrays. The second example, however, just seems weird: this is .*, not .**.
The attached patch fixes it by passing the next node to `executeAnyItem()` (via `executeItemUnwrapTargetArray()`) and then properly setting `jperOk` when `executeAnyItem()` finds values and there is no current (next) node.
I took this approach given what appears to be the intended behavior or $* on arrays in lax mode. However, I could see an argument that .* should not apply to arrays at all. If so, I can submit a new patch removing the branch that unwraps an array with .*.
Best,
David
Attachments:
0001-Fix-behavior-of-jsonpath-.-on-arrays.patchapplication/octet-stream; name=0001-Fix-behavior-of-jsonpath-.-on-arrays.patch; x-unix-mode=0644Download
From 12df9d09035a29239fd93ca600f25a902a19cd06 Mon Sep 17 00:00:00 2001
From: "David E. Wheeler" <david@justatheory.com>
Date: Tue, 4 Jun 2024 12:00:25 -0400
Subject: [PATCH] Fix behavior of jsonpath `.*` on arrays
The behavior of the `.*` jpiAnyKey jsonpath selector seems incorrect:
```
select jsonb_path_query('[1,2,3]', '$.*');
jsonb_path_query
------------------
(0 rows)
select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', '$.*');
jsonb_path_query
------------------
[3, 4, 5]
```
The first example might be expected, since `.*` is intended for object
keys, but the handing of `jpiAnyKey` has a branch for unwrapping arrays.
The second example, however, just seems weird: this is `.*`, not `.**`.
Fix it by passing the next node to `executeAnyItem()` (via
`executeItemUnwrapTargetArray()`) and then properly set `jperOk` when
`executeAnyItem()` finds values when there is no current (next) node.
While at it, document a couple functions.
---
src/backend/utils/adt/jsonpath_exec.c | 17 ++++++---
src/test/regress/expected/jsonb_jsonpath.out | 36 ++++++++++++++++++++
src/test/regress/sql/jsonb_jsonpath.sql | 6 ++++
3 files changed, 55 insertions(+), 4 deletions(-)
diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index 8a0a2dbc85..f09cd39342 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -864,8 +864,10 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt, JsonPathItem *jsp,
jb->val.binary.data, found, 1, 1, 1,
false, jspAutoUnwrap(cxt));
}
- else if (unwrap && JsonbType(jb) == jbvArray)
- return executeItemUnwrapTargetArray(cxt, jsp, jb, found, false);
+ else if (unwrap && JsonbType(jb) == jbvArray) {
+ bool hasNext = jspGetNext(jsp, &elem);
+ return executeItemUnwrapTargetArray(cxt, hasNext ? &elem : NULL, jb, found, false);
+ }
else if (!jspIgnoreStructuralErrors(cxt))
{
Assert(found);
@@ -2002,8 +2004,10 @@ executeAnyItem(JsonPathExecContext *cxt, JsonPathItem *jsp, JsonbContainer *jbc,
if (res == jperOk && !found)
break;
}
- else if (found)
+ else if (found) {
JsonValueListAppend(found, copyJsonbValue(&v));
+ res = jperOk;
+ }
else
return jperOk;
}
@@ -2976,7 +2980,8 @@ getJsonPathItem(JsonPathExecContext *cxt, JsonPathItem *item,
}
/*
- * Returns the computed value of a JSON path variable with given name.
+ * Definition of JsonPathGetVarCallback for when JsonPathExecContext.vars
+ * is specified as a List value.
*/
static JsonbValue *
GetJsonPathVar(void *cxt, char *varName, int varNameLen,
@@ -3022,6 +3027,10 @@ GetJsonPathVar(void *cxt, char *varName, int varNameLen,
return result;
}
+/*
+ * Definition of JsonPathCountVarsCallback for when JsonPathExecContext.vars
+ * is specified as a List value.
+ */
static int
CountJsonPathVars(void *cxt)
{
diff --git a/src/test/regress/expected/jsonb_jsonpath.out b/src/test/regress/expected/jsonb_jsonpath.out
index c3f8e8249d..539884ead4 100644
--- a/src/test/regress/expected/jsonb_jsonpath.out
+++ b/src/test/regress/expected/jsonb_jsonpath.out
@@ -245,6 +245,42 @@ select jsonb_path_exists('[{"a": 1}, {"a": 2}, 3]', 'strict $[*].a', silent => t
(1 row)
+select jsonb_path_query('{"a": [1,2,3], "b": [3,4,5]}', '$.*');
+ jsonb_path_query
+------------------
+ [1, 2, 3]
+ [3, 4, 5]
+(2 rows)
+
+select jsonb_path_query('[1,2,3]', '$.*');
+ jsonb_path_query
+------------------
+ 1
+ 2
+ 3
+(3 rows)
+
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', '$.*');
+ jsonb_path_query
+------------------
+ 1
+ 2
+ 3
+ {"b": [3, 4, 5]}
+(4 rows)
+
+select jsonb '{"a": [1,2,3], "b": [3,4,5]}' @? '$.*';
+ ?column?
+----------
+ t
+(1 row)
+
+select jsonb '[1,2,3,{"b": [3,4,5]}]' @? '$.*';
+ ?column?
+----------
+ t
+(1 row)
+
select jsonb_path_query('1', 'lax $.a');
jsonb_path_query
------------------
diff --git a/src/test/regress/sql/jsonb_jsonpath.sql b/src/test/regress/sql/jsonb_jsonpath.sql
index cbd2db533d..e650da10b6 100644
--- a/src/test/regress/sql/jsonb_jsonpath.sql
+++ b/src/test/regress/sql/jsonb_jsonpath.sql
@@ -44,6 +44,12 @@ select jsonb_path_exists('[{"a": 1}, {"a": 2}, 3]', 'lax $[*].a', silent => true
select jsonb_path_exists('[{"a": 1}, {"a": 2}, 3]', 'strict $[*].a', silent => false);
select jsonb_path_exists('[{"a": 1}, {"a": 2}, 3]', 'strict $[*].a', silent => true);
+select jsonb_path_query('{"a": [1,2,3], "b": [3,4,5]}', '$.*');
+select jsonb_path_query('[1,2,3]', '$.*');
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', '$.*');
+select jsonb '{"a": [1,2,3], "b": [3,4,5]}' @? '$.*';
+select jsonb '[1,2,3,{"b": [3,4,5]}]' @? '$.*';
+
select jsonb_path_query('1', 'lax $.a');
select jsonb_path_query('1', 'strict $.a');
select jsonb_path_query('1', 'strict $.*');
--
2.45.2
On Tuesday, June 4, 2024, David E. Wheeler <david@justatheory.com> wrote:
Hackers,
The behavior of the .* jpiAnyKey jsonpath selector seems incorrect.
```
select jsonb_path_query('[1,2,3]', '$.*');
jsonb_path_query
------------------
(0 rows)select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', '$.*');
jsonb_path_query
------------------
[3, 4, 5]
```The first example might be expected, since .* is intended for object keys,
but the handing of `jpiAnyKey` has a branch for unwrapping arrays. The
second example, however, just seems weird: this is .*, not .**.
This seems to be working correctly. Lax mode causes the first array level
to unwrap and produce new context item values. Then the wildcard member
accessor is applied to each. Numbers don’t have members so no matches
exist in the first example. The object in the second indeed has a single
member and so matches the wildcard and its value, the array, is returned.
David J.
On Jun 4, 2024, at 12:28 PM, David G. Johnston <david.g.johnston@gmail.com> wrote:
This seems to be working correctly. Lax mode causes the first array level to unwrap and produce new context item values. Then the wildcard member accessor is applied to each. Numbers don’t have members so no matches exist in the first example. The object in the second indeed has a single member and so matches the wildcard and its value, the array, is returned.
Oh FFS, unwrapping still breaks my brain. You’re right, of course. Here’s a new patch that demonstrates that behavior, since that code path is not currently represented in tests AFAICT (I would have expected to have broken it with this patch).
D
Attachments:
0001-Add-tests-for-jsonpath-.-on-arrays.patchapplication/octet-stream; name=0001-Add-tests-for-jsonpath-.-on-arrays.patch; x-unix-mode=0644Download
From 8cd8e82676f1cb1ea9eba9db3df6ce3f73650ef3 Mon Sep 17 00:00:00 2001
From: "David E. Wheeler" <david@justatheory.com>
Date: Tue, 4 Jun 2024 20:35:43 -0400
Subject: [PATCH] Add tests for jsonpath `.*` on arrays
There was no coverage for the path to unwrap an array before applying
`.*` to it, so add tests that explicitly test `.*` for both objects and
arrays, showing how no results are returned for an array of scalars, but
results are returned when the array contains an object. Also test the
behavior in strict mode and with the `@?` operator.
While at it, teach `executeAnyItem()` to return `jperOk` when `found`
exist, not because it will be used (the result and `found` are inspected
by different functions), but because it seems like the proper thing to
return from `executeAnyItem()` when considered in isolation.
Unrelated but potentially useful to future source readers: document
`GetJsonPathVar` and `CountJsonPathVars`.
---
src/backend/utils/adt/jsonpath_exec.c | 11 ++++-
src/test/regress/expected/jsonb_jsonpath.out | 44 ++++++++++++++++++++
src/test/regress/sql/jsonb_jsonpath.sql | 9 ++++
3 files changed, 62 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index 8a0a2dbc85..5ef168e978 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -2002,8 +2002,10 @@ executeAnyItem(JsonPathExecContext *cxt, JsonPathItem *jsp, JsonbContainer *jbc,
if (res == jperOk && !found)
break;
}
- else if (found)
+ else if (found) {
JsonValueListAppend(found, copyJsonbValue(&v));
+ res = jperOk;
+ }
else
return jperOk;
}
@@ -2976,7 +2978,8 @@ getJsonPathItem(JsonPathExecContext *cxt, JsonPathItem *item,
}
/*
- * Returns the computed value of a JSON path variable with given name.
+ * Definition of JsonPathGetVarCallback for when JsonPathExecContext.vars
+ * is specified as a List value.
*/
static JsonbValue *
GetJsonPathVar(void *cxt, char *varName, int varNameLen,
@@ -3022,6 +3025,10 @@ GetJsonPathVar(void *cxt, char *varName, int varNameLen,
return result;
}
+/*
+ * Definition of JsonPathCountVarsCallback for when JsonPathExecContext.vars
+ * is specified as a List value.
+ */
static int
CountJsonPathVars(void *cxt)
{
diff --git a/src/test/regress/expected/jsonb_jsonpath.out b/src/test/regress/expected/jsonb_jsonpath.out
index c3f8e8249d..7ec4a0ef38 100644
--- a/src/test/regress/expected/jsonb_jsonpath.out
+++ b/src/test/regress/expected/jsonb_jsonpath.out
@@ -245,6 +245,50 @@ select jsonb_path_exists('[{"a": 1}, {"a": 2}, 3]', 'strict $[*].a', silent => t
(1 row)
+select jsonb_path_query('{"a": [1,2,3], "b": [3,4,5]}', '$.*');
+ jsonb_path_query
+------------------
+ [1, 2, 3]
+ [3, 4, 5]
+(2 rows)
+
+select jsonb_path_query('[1,2,3]', '$.*');
+ jsonb_path_query
+------------------
+(0 rows)
+
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', '$.*');
+ jsonb_path_query
+------------------
+ [3, 4, 5]
+(1 row)
+
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'strict $.*');
+ERROR: jsonpath wildcard member accessor can only be applied to an object
+select jsonb '{"a": [1,2,3], "b": [3,4,5]}' @? '$.*';
+ ?column?
+----------
+ t
+(1 row)
+
+select jsonb '[1,2,3]' @? '$.*';
+ ?column?
+----------
+ f
+(1 row)
+
+select jsonb '[1,2,3,{"b": [3,4,5]}]' @? '$.*';
+ ?column?
+----------
+ t
+(1 row)
+
+select jsonb '[1,2,3,{"b": [3,4,5]}]' @? 'strict $.*';
+ ?column?
+----------
+
+(1 row)
+
select jsonb_path_query('1', 'lax $.a');
jsonb_path_query
------------------
diff --git a/src/test/regress/sql/jsonb_jsonpath.sql b/src/test/regress/sql/jsonb_jsonpath.sql
index cbd2db533d..fa6f3cd5f9 100644
--- a/src/test/regress/sql/jsonb_jsonpath.sql
+++ b/src/test/regress/sql/jsonb_jsonpath.sql
@@ -44,6 +44,15 @@ select jsonb_path_exists('[{"a": 1}, {"a": 2}, 3]', 'lax $[*].a', silent => true
select jsonb_path_exists('[{"a": 1}, {"a": 2}, 3]', 'strict $[*].a', silent => false);
select jsonb_path_exists('[{"a": 1}, {"a": 2}, 3]', 'strict $[*].a', silent => true);
+select jsonb_path_query('{"a": [1,2,3], "b": [3,4,5]}', '$.*');
+select jsonb_path_query('[1,2,3]', '$.*');
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', '$.*');
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'strict $.*');
+select jsonb '{"a": [1,2,3], "b": [3,4,5]}' @? '$.*';
+select jsonb '[1,2,3]' @? '$.*';
+select jsonb '[1,2,3,{"b": [3,4,5]}]' @? '$.*';
+select jsonb '[1,2,3,{"b": [3,4,5]}]' @? 'strict $.*';
+
select jsonb_path_query('1', 'lax $.a');
select jsonb_path_query('1', 'strict $.a');
select jsonb_path_query('1', 'strict $.*');
--
2.45.2
On Jun 4, 2024, at 20:45, David E. Wheeler <david@justatheory.com> wrote:
Here’s a new patch that demonstrates that behavior, since that code path is not currently represented in tests AFAICT (I would have expected to have broken it with this patch).
Commitfest link:
https://commitfest.postgresql.org/48/5017/
D
On Jun 4, 2024, at 20:45, David E. Wheeler <david@justatheory.com> wrote:
Oh FFS, unwrapping still breaks my brain. You’re right, of course. Here’s a new patch that demonstrates that behavior, since that code path is not currently represented in tests AFAICT (I would have expected to have broken it with this patch).
Rebased and moved the new tests to the end of the file.
D
Attachments:
v2-0001-Add-tests-for-jsonpath-.-on-arrays.patchapplication/octet-stream; name=v2-0001-Add-tests-for-jsonpath-.-on-arrays.patch; x-unix-mode=0644Download
From 8cd8e82676f1cb1ea9eba9db3df6ce3f73650ef3 Mon Sep 17 00:00:00 2001
From: "David E. Wheeler" <david@justatheory.com>
Date: Tue, 4 Jun 2024 20:35:43 -0400
Subject: [PATCH v2] Add tests for jsonpath `.*` on arrays
There was no coverage for the path to unwrap an array before applying
`.*` to it, so add tests that explicitly test `.*` for both objects and
arrays, showing how no results are returned for an array of scalars, but
results are returned when the array contains an object. Also test the
behavior in strict mode and with the `@?` operator.
While at it, teach `executeAnyItem()` to return `jperOk` when `found`
exist, not because it will be used (the result and `found` are inspected
by different functions), but because it seems like the proper thing to
return from `executeAnyItem()` when considered in isolation.
Unrelated but potentially useful to future source readers: document
`GetJsonPathVar` and `CountJsonPathVars`.
---
src/backend/utils/adt/jsonpath_exec.c | 11 ++++-
src/test/regress/expected/jsonb_jsonpath.out | 44 ++++++++++++++++++++
src/test/regress/sql/jsonb_jsonpath.sql | 9 ++++
3 files changed, 62 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index 8a0a2dbc85..5ef168e978 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -2002,8 +2002,10 @@ executeAnyItem(JsonPathExecContext *cxt, JsonPathItem *jsp, JsonbContainer *jbc,
if (res == jperOk && !found)
break;
}
- else if (found)
+ else if (found) {
JsonValueListAppend(found, copyJsonbValue(&v));
+ res = jperOk;
+ }
else
return jperOk;
}
@@ -2976,7 +2978,8 @@ getJsonPathItem(JsonPathExecContext *cxt, JsonPathItem *item,
}
/*
- * Returns the computed value of a JSON path variable with given name.
+ * Definition of JsonPathGetVarCallback for when JsonPathExecContext.vars
+ * is specified as a List value.
*/
static JsonbValue *
GetJsonPathVar(void *cxt, char *varName, int varNameLen,
@@ -3022,6 +3025,10 @@ GetJsonPathVar(void *cxt, char *varName, int varNameLen,
return result;
}
+/*
+ * Definition of JsonPathCountVarsCallback for when JsonPathExecContext.vars
+ * is specified as a List value.
+ */
static int
CountJsonPathVars(void *cxt)
{
diff --git a/src/test/regress/expected/jsonb_jsonpath.out b/src/test/regress/expected/jsonb_jsonpath.out
index c3f8e8249d..7ec4a0ef38 100644
--- a/src/test/regress/expected/jsonb_jsonpath.out
+++ b/src/test/regress/expected/jsonb_jsonpath.out
@@ -245,6 +245,50 @@ select jsonb_path_exists('[{"a": 1}, {"a": 2}, 3]', 'strict $[*].a', silent => t
(1 row)
+select jsonb_path_query('{"a": [1,2,3], "b": [3,4,5]}', '$.*');
+ jsonb_path_query
+------------------
+ [1, 2, 3]
+ [3, 4, 5]
+(2 rows)
+
+select jsonb_path_query('[1,2,3]', '$.*');
+ jsonb_path_query
+------------------
+(0 rows)
+
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', '$.*');
+ jsonb_path_query
+------------------
+ [3, 4, 5]
+(1 row)
+
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'strict $.*');
+ERROR: jsonpath wildcard member accessor can only be applied to an object
+select jsonb '{"a": [1,2,3], "b": [3,4,5]}' @? '$.*';
+ ?column?
+----------
+ t
+(1 row)
+
+select jsonb '[1,2,3]' @? '$.*';
+ ?column?
+----------
+ f
+(1 row)
+
+select jsonb '[1,2,3,{"b": [3,4,5]}]' @? '$.*';
+ ?column?
+----------
+ t
+(1 row)
+
+select jsonb '[1,2,3,{"b": [3,4,5]}]' @? 'strict $.*';
+ ?column?
+----------
+
+(1 row)
+
select jsonb_path_query('1', 'lax $.a');
jsonb_path_query
------------------
diff --git a/src/test/regress/sql/jsonb_jsonpath.sql b/src/test/regress/sql/jsonb_jsonpath.sql
index cbd2db533d..fa6f3cd5f9 100644
--- a/src/test/regress/sql/jsonb_jsonpath.sql
+++ b/src/test/regress/sql/jsonb_jsonpath.sql
@@ -44,6 +44,15 @@ select jsonb_path_exists('[{"a": 1}, {"a": 2}, 3]', 'lax $[*].a', silent => true
select jsonb_path_exists('[{"a": 1}, {"a": 2}, 3]', 'strict $[*].a', silent => false);
select jsonb_path_exists('[{"a": 1}, {"a": 2}, 3]', 'strict $[*].a', silent => true);
+select jsonb_path_query('{"a": [1,2,3], "b": [3,4,5]}', '$.*');
+select jsonb_path_query('[1,2,3]', '$.*');
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', '$.*');
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'strict $.*');
+select jsonb '{"a": [1,2,3], "b": [3,4,5]}' @? '$.*';
+select jsonb '[1,2,3]' @? '$.*';
+select jsonb '[1,2,3,{"b": [3,4,5]}]' @? '$.*';
+select jsonb '[1,2,3,{"b": [3,4,5]}]' @? 'strict $.*';
+
select jsonb_path_query('1', 'lax $.a');
select jsonb_path_query('1', 'strict $.a');
select jsonb_path_query('1', 'strict $.*');
--
2.45.2
On Jun 7, 2024, at 10:23, David E. Wheeler <david@justatheory.com> wrote:
Rebased and moved the new tests to the end of the file.
Bah, sorry, that was the previous patch. Here’s v3.
D
Attachments:
v3-0001-Add-tests-for-jsonpath-.-on-arrays.patchapplication/octet-stream; name=v3-0001-Add-tests-for-jsonpath-.-on-arrays.patch; x-unix-mode=0644Download
From 403ab2f7da315069017f22ebd46bb5eb38ae1f4e Mon Sep 17 00:00:00 2001
From: "David E. Wheeler" <david@justatheory.com>
Date: Fri, 7 Jun 2024 10:21:06 -0400
Subject: [PATCH v3] Add tests for jsonpath `.*` on arrays
There was no coverage for the path to unwrap an array before applying
`.*` to it, so add tests that explicitly test `.*` for both objects and
arrays, showing how no results are returned for an array of scalars, but
results are returned when the array contains an object. Also test the
behavior in strict mode and with the `@?` operator.
While at it, teach `executeAnyItem()` to return `jperOk` when `found`
exist, not because it will be used (the result and `found` are inspected
by different functions), but because it seems like the proper thing to
return from `executeAnyItem()` when considered in isolation.
Unrelated but potentially useful to future source readers: document
`GetJsonPathVar` and `CountJsonPathVars`.
---
src/backend/utils/adt/jsonpath_exec.c | 11 ++++-
src/test/regress/expected/jsonb_jsonpath.out | 45 ++++++++++++++++++++
src/test/regress/sql/jsonb_jsonpath.sql | 10 +++++
3 files changed, 64 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index 8a0a2dbc85..5ef168e978 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -2002,8 +2002,10 @@ executeAnyItem(JsonPathExecContext *cxt, JsonPathItem *jsp, JsonbContainer *jbc,
if (res == jperOk && !found)
break;
}
- else if (found)
+ else if (found) {
JsonValueListAppend(found, copyJsonbValue(&v));
+ res = jperOk;
+ }
else
return jperOk;
}
@@ -2976,7 +2978,8 @@ getJsonPathItem(JsonPathExecContext *cxt, JsonPathItem *item,
}
/*
- * Returns the computed value of a JSON path variable with given name.
+ * Definition of JsonPathGetVarCallback for when JsonPathExecContext.vars
+ * is specified as a List value.
*/
static JsonbValue *
GetJsonPathVar(void *cxt, char *varName, int varNameLen,
@@ -3022,6 +3025,10 @@ GetJsonPathVar(void *cxt, char *varName, int varNameLen,
return result;
}
+/*
+ * Definition of JsonPathCountVarsCallback for when JsonPathExecContext.vars
+ * is specified as a List value.
+ */
static int
CountJsonPathVars(void *cxt)
{
diff --git a/src/test/regress/expected/jsonb_jsonpath.out b/src/test/regress/expected/jsonb_jsonpath.out
index c3f8e8249d..0611830842 100644
--- a/src/test/regress/expected/jsonb_jsonpath.out
+++ b/src/test/regress/expected/jsonb_jsonpath.out
@@ -4384,3 +4384,48 @@ ORDER BY s1.num, s2.num;
{"s": "B"} | {"s": "B"} | false | true | true | true | false
(144 rows)
+-- Test any key on arrays with and without unwrapping.
+select jsonb_path_query('{"a": [1,2,3], "b": [3,4,5]}', '$.*');
+ jsonb_path_query
+------------------
+ [1, 2, 3]
+ [3, 4, 5]
+(2 rows)
+
+select jsonb_path_query('[1,2,3]', '$.*');
+ jsonb_path_query
+------------------
+(0 rows)
+
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', '$.*');
+ jsonb_path_query
+------------------
+ [3, 4, 5]
+(1 row)
+
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'strict $.*');
+ERROR: jsonpath wildcard member accessor can only be applied to an object
+select jsonb '{"a": [1,2,3], "b": [3,4,5]}' @? '$.*';
+ ?column?
+----------
+ t
+(1 row)
+
+select jsonb '[1,2,3]' @? '$.*';
+ ?column?
+----------
+ f
+(1 row)
+
+select jsonb '[1,2,3,{"b": [3,4,5]}]' @? '$.*';
+ ?column?
+----------
+ t
+(1 row)
+
+select jsonb '[1,2,3,{"b": [3,4,5]}]' @? 'strict $.*';
+ ?column?
+----------
+
+(1 row)
+
diff --git a/src/test/regress/sql/jsonb_jsonpath.sql b/src/test/regress/sql/jsonb_jsonpath.sql
index cbd2db533d..4f6139b7ef 100644
--- a/src/test/regress/sql/jsonb_jsonpath.sql
+++ b/src/test/regress/sql/jsonb_jsonpath.sql
@@ -1115,3 +1115,13 @@ SELECT
jsonb_path_query_first(s1.j, '$.s > $s', vars => s2.j) gt
FROM str s1, str s2
ORDER BY s1.num, s2.num;
+
+-- Test any key on arrays with and without unwrapping.
+select jsonb_path_query('{"a": [1,2,3], "b": [3,4,5]}', '$.*');
+select jsonb_path_query('[1,2,3]', '$.*');
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', '$.*');
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'strict $.*');
+select jsonb '{"a": [1,2,3], "b": [3,4,5]}' @? '$.*';
+select jsonb '[1,2,3]' @? '$.*';
+select jsonb '[1,2,3,{"b": [3,4,5]}]' @? '$.*';
+select jsonb '[1,2,3,{"b": [3,4,5]}]' @? 'strict $.*';
--
2.45.2
Вторник, 25 июня 2024, 11:17 +07:00 от David E. Wheeler <david@justatheory.com>:
On Jun 7, 2024, at 10:23, David E. Wheeler < david@justatheory.com > wrote:
Rebased and moved the new tests to the end of the file.
Bah, sorry, that was the previous patch. Here’s v3.
D
Hi! Looks good to me, but I have several comments.
Your patch improves tests, but why did you change formatting in jsonpath_exec.c? What's the motivation?
[1]: select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'strict $.*'); I propose adding a similar test with explicitly specified lax mode: select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'lax $.*'); to show what lax mode is set by default. Odd empty result for the test: select jsonb '[1,2,3,{"b": [3,4,5]}]' @? 'strict $.*'; I expected an error like in test [1]. This behavior is not obvious to me. Everything else is cool. Thanks to the patch and the discussion above, I began to understand better how wildcards in JSON work. Best regards, Stepan Neretin.
I propose adding a similar test with explicitly specified lax mode: select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'lax $.*'); to show what lax mode is set by default.
Odd empty result for the test: select jsonb '[1,2,3,{"b": [3,4,5]}]' @? 'strict $.*';
I expected an error like in test [1]select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'strict $.*'); I propose adding a similar test with explicitly specified lax mode: select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'lax $.*'); to show what lax mode is set by default. Odd empty result for the test: select jsonb '[1,2,3,{"b": [3,4,5]}]' @? 'strict $.*'; I expected an error like in test [1]. This behavior is not obvious to me. Everything else is cool. Thanks to the patch and the discussion above, I began to understand better how wildcards in JSON work. Best regards, Stepan Neretin. . This behavior is not obvious to me.
Everything else is cool. Thanks to the patch and the discussion above, I began to understand better how wildcards in JSON work.
Best regards, Stepan Neretin.
On Jun 25, 2024, at 12:46 AM, Степан Неретин <fenixrnd@mail.ru> wrote:
Hi! Looks good to me, but I have several comments.
Thanks for your review!
Your patch improves tests, but why did you change formatting in jsonpath_exec.c? What's the motivation?
It’s not just formatting. From the commit message:
While at it, teach `executeAnyItem()` to return `jperOk` when `found`
exist, not because it will be used (the result and `found` are inspected
by different functions), but because it seems like the proper thing to
return from `executeAnyItem()` when considered in isolation.
I have since realized it’s not a complete fix for the issue, and hacked around it in my Go version. Would be fine to remove that bit, but IIRC this was the only execution function that would return `jperNotFound` when it in fact adds items to the `found` list. The current implementation only looks at one or the other, so it’s not super important, but I found the inconsistency annoying and sometimes confusing.
[1] select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'strict $.*');
I propose adding a similar test with explicitly specified lax mode: select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'lax $.*'); to show what lax mode is set by default.
Very few of the other tests do so; I can add it if it’s important for this case, though.
Odd empty result for the test: select jsonb '[1,2,3,{"b": [3,4,5]}]' @? 'strict $.*';
I expected an error like in test [1]. This behavior is not obvious to me.
@? suppresses a number of errors. Perhaps I should add a variant of the error-raising query that passes the silent arg, which would also suppress the error.
Everything else is cool. Thanks to the patch and the discussion above, I began to understand better how wildcards in JSON work.
Yeah, it’s kind of wild TBH.
Best,
David
On Jun 25, 2024, at 13:48, David E. Wheeler <david@justatheory.com> wrote:
I have since realized it’s not a complete fix for the issue, and hacked around it in my Go version. Would be fine to remove that bit, but IIRC this was the only execution function that would return `jperNotFound` when it in fact adds items to the `found` list. The current implementation only looks at one or the other, so it’s not super important, but I found the inconsistency annoying and sometimes confusing.
I’ve removed this change.
[1] select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'strict $.*');
I propose adding a similar test with explicitly specified lax mode: select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'lax $.*'); to show what lax mode is set by default.Very few of the other tests do so; I can add it if it’s important for this case, though.
Went ahead and added lax.
@? suppresses a number of errors. Perhaps I should add a variant of the error-raising query that passes the silent arg, which would also suppress the error.
Added a variant where the silent param suppresses the error, too.
V2 attached and the PR updated:
https://github.com/theory/postgres/pull/4/files
Best,
David
Attachments:
v2-0001-Add-tests-for-jsonpath-.-on-arrays.patchapplication/octet-stream; name=v2-0001-Add-tests-for-jsonpath-.-on-arrays.patch; x-unix-mode=0644Download
From a7f9c1e4261d4279a4b38ba8ef903cc0e0707ca5 Mon Sep 17 00:00:00 2001
From: "David E. Wheeler" <david@justatheory.com>
Date: Wed, 26 Jun 2024 14:12:47 -0400
Subject: [PATCH v2] Add tests for jsonpath `.*` on arrays
There was no coverage for the path to unwrap an array before applying
`.*` to it, so add tests that explicitly test `.*` for both objects and
arrays, showing how no results are returned for an array of scalars, but
results are returned when the array contains an object. Also test the
behavior in strict mode with and without the silent parameter, and with
the `@?` operator.
Unrelated but potentially useful to future source readers: document
`GetJsonPathVar` and `CountJsonPathVars`.
---
src/backend/utils/adt/jsonpath_exec.c | 7 ++-
src/test/regress/expected/jsonb_jsonpath.out | 50 ++++++++++++++++++++
src/test/regress/sql/jsonb_jsonpath.sql | 11 +++++
3 files changed, 67 insertions(+), 1 deletion(-)
diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index d79c929822..bf69f0d824 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -2978,7 +2978,8 @@ getJsonPathItem(JsonPathExecContext *cxt, JsonPathItem *item,
}
/*
- * Returns the computed value of a JSON path variable with given name.
+ * Definition of JsonPathGetVarCallback for when JsonPathExecContext.vars
+ * is specified as a List value.
*/
static JsonbValue *
GetJsonPathVar(void *cxt, char *varName, int varNameLen,
@@ -3025,6 +3026,10 @@ GetJsonPathVar(void *cxt, char *varName, int varNameLen,
return result;
}
+/*
+ * Definition of JsonPathCountVarsCallback for when JsonPathExecContext.vars
+ * is specified as a List value.
+ */
static int
CountJsonPathVars(void *cxt)
{
diff --git a/src/test/regress/expected/jsonb_jsonpath.out b/src/test/regress/expected/jsonb_jsonpath.out
index a6112e86fa..3dea937887 100644
--- a/src/test/regress/expected/jsonb_jsonpath.out
+++ b/src/test/regress/expected/jsonb_jsonpath.out
@@ -4394,3 +4394,53 @@ ORDER BY s1.num, s2.num;
{"s": "B"} | {"s": "B"} | false | true | true | true | false
(144 rows)
+-- Test any key on arrays with and without unwrapping.
+select jsonb_path_query('{"a": [1,2,3], "b": [3,4,5]}', '$.*');
+ jsonb_path_query
+------------------
+ [1, 2, 3]
+ [3, 4, 5]
+(2 rows)
+
+select jsonb_path_query('[1,2,3]', '$.*');
+ jsonb_path_query
+------------------
+(0 rows)
+
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'lax $.*');
+ jsonb_path_query
+------------------
+ [3, 4, 5]
+(1 row)
+
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'strict $.*');
+ERROR: jsonpath wildcard member accessor can only be applied to an object
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'strict $.*', NULL, true);
+ jsonb_path_query
+------------------
+(0 rows)
+
+select jsonb '{"a": [1,2,3], "b": [3,4,5]}' @? '$.*';
+ ?column?
+----------
+ t
+(1 row)
+
+select jsonb '[1,2,3]' @? '$.*';
+ ?column?
+----------
+ f
+(1 row)
+
+select jsonb '[1,2,3,{"b": [3,4,5]}]' @? 'lax $.*';
+ ?column?
+----------
+ t
+(1 row)
+
+select jsonb '[1,2,3,{"b": [3,4,5]}]' @? 'strict $.*';
+ ?column?
+----------
+
+(1 row)
+
diff --git a/src/test/regress/sql/jsonb_jsonpath.sql b/src/test/regress/sql/jsonb_jsonpath.sql
index 5e14f7759b..f60bccc654 100644
--- a/src/test/regress/sql/jsonb_jsonpath.sql
+++ b/src/test/regress/sql/jsonb_jsonpath.sql
@@ -1116,3 +1116,14 @@ SELECT
jsonb_path_query_first(s1.j, '$.s > $s', vars => s2.j) gt
FROM str s1, str s2
ORDER BY s1.num, s2.num;
+
+-- Test any key on arrays with and without unwrapping.
+select jsonb_path_query('{"a": [1,2,3], "b": [3,4,5]}', '$.*');
+select jsonb_path_query('[1,2,3]', '$.*');
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'lax $.*');
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'strict $.*');
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'strict $.*', NULL, true);
+select jsonb '{"a": [1,2,3], "b": [3,4,5]}' @? '$.*';
+select jsonb '[1,2,3]' @? '$.*';
+select jsonb '[1,2,3,{"b": [3,4,5]}]' @? 'lax $.*';
+select jsonb '[1,2,3,{"b": [3,4,5]}]' @? 'strict $.*';
--
2.45.2
On Thu, Jun 27, 2024 at 1:16 AM David E. Wheeler <david@justatheory.com>
wrote:
On Jun 25, 2024, at 13:48, David E. Wheeler <david@justatheory.com> wrote:
I have since realized it’s not a complete fix for the issue, and hacked
around it in my Go version. Would be fine to remove that bit, but IIRC this
was the only execution function that would return `jperNotFound` when it in
fact adds items to the `found` list. The current implementation only looks
at one or the other, so it’s not super important, but I found the
inconsistency annoying and sometimes confusing.I’ve removed this change.
[1] select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'strict $.*');
I propose adding a similar test with explicitly specified lax mode:select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'lax $.*'); to show what
lax mode is set by default.Very few of the other tests do so; I can add it if it’s important for
this case, though.
Went ahead and added lax.
@? suppresses a number of errors. Perhaps I should add a variant of the
error-raising query that passes the silent arg, which would also suppress
the error.Added a variant where the silent param suppresses the error, too.
V2 attached and the PR updated:
https://github.com/theory/postgres/pull/4/files
Best,
David
HI! Now it looks good for me.
Best regards, Stepan Neretin.
On Thu, Jun 27, 2024 at 11:53:14AM +0700, Stepan Neretin wrote:
HI! Now it looks good for me.
The tests of jsonb_jsonpath.sql include a lot of patterns for @? and
jsonb_path_query with the lax and strict modes, so shouldn't these
additions be grouped closer to the existing tests rather than added at
the end of the file?
--
Michael
On Jun 27, 2024, at 04:17, Michael Paquier <michael@paquier.xyz> wrote:
The tests of jsonb_jsonpath.sql include a lot of patterns for @? and
jsonb_path_query with the lax and strict modes, so shouldn't these
additions be grouped closer to the existing tests rather than added at
the end of the file?
I think you could argue that they should go with other tests for array unwrapping, though it’s kind of mixed throughout. But that’s more the bit I was testing; almost all the tests are lax, and it’s less the strict behavior to test here than the explicit behavior of unwrapping in lax mode.
But ultimately I don’t care where they go, just that we have them.
D
On Jun 27, 2024, at 04:17, Michael Paquier <michael@paquier.xyz> wrote:
The tests of jsonb_jsonpath.sql include a lot of patterns for @? and
jsonb_path_query with the lax and strict modes, so shouldn't these
additions be grouped closer to the existing tests rather than added at
the end of the file?
I’ve moved them closer to other tests for unwrapping behavior in the attached updated and rebased v3 patch.
Best,
David
Attachments:
v3-0001-Add-tests-for-jsonpath-.-on-arrays.patchapplication/octet-stream; name=v3-0001-Add-tests-for-jsonpath-.-on-arrays.patch; x-unix-mode=0644Download
From eefe7a08e7c365082954c0026a13dad41f713744 Mon Sep 17 00:00:00 2001
From: "David E. Wheeler" <david@justatheory.com>
Date: Mon, 8 Jul 2024 12:08:00 -0400
Subject: [PATCH v3] Add tests for jsonpath `.*` on arrays
There was no coverage for the path to unwrap an array before applying
`.*` to it, so add tests that explicitly test `.*` for both objects and
arrays, showing how no results are returned for an array of scalars, but
results are returned when the array contains an object. Also test the
behavior in strict mode with and without the silent parameter, and with
the `@?` operator.
Unrelated but potentially useful to future source readers: document
`GetJsonPathVar` and `CountJsonPathVars`.
---
src/backend/utils/adt/jsonpath_exec.c | 7 ++-
src/test/regress/expected/jsonb_jsonpath.out | 50 ++++++++++++++++++++
src/test/regress/sql/jsonb_jsonpath.sql | 11 +++++
3 files changed, 67 insertions(+), 1 deletion(-)
diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index d79c929822..bf69f0d824 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -2978,7 +2978,8 @@ getJsonPathItem(JsonPathExecContext *cxt, JsonPathItem *item,
}
/*
- * Returns the computed value of a JSON path variable with given name.
+ * Definition of JsonPathGetVarCallback for when JsonPathExecContext.vars
+ * is specified as a List value.
*/
static JsonbValue *
GetJsonPathVar(void *cxt, char *varName, int varNameLen,
@@ -3025,6 +3026,10 @@ GetJsonPathVar(void *cxt, char *varName, int varNameLen,
return result;
}
+/*
+ * Definition of JsonPathCountVarsCallback for when JsonPathExecContext.vars
+ * is specified as a List value.
+ */
static int
CountJsonPathVars(void *cxt)
{
diff --git a/src/test/regress/expected/jsonb_jsonpath.out b/src/test/regress/expected/jsonb_jsonpath.out
index a6112e86fa..7bb4eb1bc2 100644
--- a/src/test/regress/expected/jsonb_jsonpath.out
+++ b/src/test/regress/expected/jsonb_jsonpath.out
@@ -1135,6 +1135,56 @@ select jsonb_path_query('{"a": [1, 2]}', 'lax $.a * 3', silent => true);
------------------
(0 rows)
+-- any key on arrays with and without unwrapping.
+select jsonb_path_query('{"a": [1,2,3], "b": [3,4,5]}', '$.*');
+ jsonb_path_query
+------------------
+ [1, 2, 3]
+ [3, 4, 5]
+(2 rows)
+
+select jsonb_path_query('[1,2,3]', '$.*');
+ jsonb_path_query
+------------------
+(0 rows)
+
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'lax $.*');
+ jsonb_path_query
+------------------
+ [3, 4, 5]
+(1 row)
+
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'strict $.*');
+ERROR: jsonpath wildcard member accessor can only be applied to an object
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'strict $.*', NULL, true);
+ jsonb_path_query
+------------------
+(0 rows)
+
+select jsonb '{"a": [1,2,3], "b": [3,4,5]}' @? '$.*';
+ ?column?
+----------
+ t
+(1 row)
+
+select jsonb '[1,2,3]' @? '$.*';
+ ?column?
+----------
+ f
+(1 row)
+
+select jsonb '[1,2,3,{"b": [3,4,5]}]' @? 'lax $.*';
+ ?column?
+----------
+ t
+(1 row)
+
+select jsonb '[1,2,3,{"b": [3,4,5]}]' @? 'strict $.*';
+ ?column?
+----------
+
+(1 row)
+
-- extension: boolean expressions
select jsonb_path_query('2', '$ > 1');
jsonb_path_query
diff --git a/src/test/regress/sql/jsonb_jsonpath.sql b/src/test/regress/sql/jsonb_jsonpath.sql
index 5e14f7759b..17f9d038c0 100644
--- a/src/test/regress/sql/jsonb_jsonpath.sql
+++ b/src/test/regress/sql/jsonb_jsonpath.sql
@@ -241,6 +241,17 @@ select jsonb_path_query('{"a": [2, 3, 4]}', 'lax -$.a');
select jsonb_path_query('{"a": [1, 2]}', 'lax $.a * 3');
select jsonb_path_query('{"a": [1, 2]}', 'lax $.a * 3', silent => true);
+-- any key on arrays with and without unwrapping.
+select jsonb_path_query('{"a": [1,2,3], "b": [3,4,5]}', '$.*');
+select jsonb_path_query('[1,2,3]', '$.*');
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'lax $.*');
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'strict $.*');
+select jsonb_path_query('[1,2,3,{"b": [3,4,5]}]', 'strict $.*', NULL, true);
+select jsonb '{"a": [1,2,3], "b": [3,4,5]}' @? '$.*';
+select jsonb '[1,2,3]' @? '$.*';
+select jsonb '[1,2,3,{"b": [3,4,5]}]' @? 'lax $.*';
+select jsonb '[1,2,3,{"b": [3,4,5]}]' @? 'strict $.*';
+
-- extension: boolean expressions
select jsonb_path_query('2', '$ > 1');
select jsonb_path_query('2', '$ <= 1');
--
2.45.2
On Mon, Jul 8, 2024 at 11:09 PM David E. Wheeler <david@justatheory.com>
wrote:
On Jun 27, 2024, at 04:17, Michael Paquier <michael@paquier.xyz> wrote:
The tests of jsonb_jsonpath.sql include a lot of patterns for @? and
jsonb_path_query with the lax and strict modes, so shouldn't these
additions be grouped closer to the existing tests rather than added at
the end of the file?I’ve moved them closer to other tests for unwrapping behavior in the
attached updated and rebased v3 patch.Best,
David
Hi! Looks good to me now! Please, register a patch in CF.
Best regards, Stepan Neretin.
On Jul 15, 2024, at 07:07, Stepan Neretin <sncfmgg@gmail.com> wrote:
Hi! Looks good to me now! Please, register a patch in CF.
Best regards, Stepan Neretin.
It’s here:
https://commitfest.postgresql.org/48/5017/
Best,
David
On Mon, Jul 15, 2024 at 10:29:32AM -0400, David E. Wheeler wrote:
It’s here:
Sorry for the delay. Finally came back to it, and applied the tests.
Thanks!
It was fun to see that HEAD was silenced with the first patch of this
thread that tweaked the behavior with arrays.
Regarding the comments, I have left them out for now. That may be a
good start, but it also feels like we should do a much better job
overall with the area of jsonpath_exec.c. One thing that may help as
a start is to reorganize the routines of the file and divide them into
sub-categories, or even go through a deeper refactoring to help
readers go through the existing 4.5k lines of code that are in this
single file..
--
Michael
On Jul 19, 2024, at 01:42, Michael Paquier <michael@paquier.xyz> wrote:
Sorry for the delay. Finally came back to it, and applied the tests.
Thanks!
Awesome, thank you!
It was fun to see that HEAD was silenced with the first patch of this
thread that tweaked the behavior with arrays.
Uh, what? Sorry I don’t follow.
Regarding the comments, I have left them out for now. That may be a
good start, but it also feels like we should do a much better job
overall with the area of jsonpath_exec.c.
I put them in because it took me a bit to track down that they were among the implementors of JsonPathGetVarCallback as I was porting the code.
One thing that may help as
a start is to reorganize the routines of the file and divide them into
sub-categories, or even go through a deeper refactoring to help
readers go through the existing 4.5k lines of code that are in this
single file..
After I got all the tests passing in the port to Go, I split it up into 14 implementation and 15 test files (with all of jsonb_jsonpath.(sql.out) in one file). Was much easier to reason about that way.
https://github.com/theory/sqljson/tree/main/path/exec
Best,
David
On Fri, Jul 19, 2024 at 09:49:50AM -0400, David E. Wheeler wrote:
It was fun to see that HEAD was silenced with the first patch of this
thread that tweaked the behavior with arrays.Uh, what? Sorry I don’t follow.
What I mean is that the main regression test suite did not complain on
your original patch posted here:
/messages/by-id/A95346F9-6147-46E0-809E-532A485D71D6@justatheory.com
But the new tests showed a difference, and that's enough for me to see
value in the new tests.
--
Michael
On Jul 21, 2024, at 20:54, Michael Paquier <michael@paquier.xyz> wrote:
What I mean is that the main regression test suite did not complain on
your original patch posted here:
/messages/by-id/A95346F9-6147-46E0-809E-532A485D71D6@justatheory.comBut the new tests showed a difference, and that's enough for me to see
value in the new tests.
Oh, got it, nice!
Thanks,
David