Re: GIN (JSONPath) checks equality only on the first node of each operand - Mailing list pgsql-bugs

Chinmay Kanchi <cgkanchi@gmail.com> writes:
> When checking equality with an operand that has additional nodes
> ("2".double()), GIN only considers the first node ("2"). So $.a ==
> "2".integer() looks up a match for the json string "2" instead of the
> number 2, and silently drops rows that a sequential scan would have
> returned.

Hmph, another bug in the same function as your previous report :-(.
Here's a less-WIP patch for both.

            regards, tom lane

From 327a2ad9221f146e9fe323e6f121db980a46ad8b Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sun, 11 Oct 2026 14:03:28 -0400
Subject: [PATCH v1] Fix two errors in extraction of GIN indexquals from
 jsonpath queries.

extract_jsp_bool_expr applied De Morgan's laws incorrectly when
considering an AND operator underneath a NOT.  If one AND input
was implementable as a GIN indexqual while the other was not,
we derived an incomplete indexqual, leading to some rows not
being returned that should be.

extract_jsp_bool_expr also failed to check for methods being
applied to literals, so that for example '$.x == "42".double()'
was treated as '$.x == "42"' not '$.x == 42'.  The indexqual
thus selected the wrong rows, all of which would fail qual
recheck, leading to no rows being returned.

While we're here, improve the comments a trifle.

Reported-by: Chinmay Kanchi <cgkanchi@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAJqPDh9Cm=x8eYZ1Jf=_avd2WEqc=0t_f7_SO4igo-CKPi25_Q@mail.gmail.com
Discussion: https://postgr.es/m/CAJqPDh-tu5KY66KU8cvAEg3DQrZLzDZwjkLLa42zD+_Xm1WJAg@mail.gmail.com
Backpatch-through: 14
---
 src/backend/utils/adt/jsonb_gin.c   | 28 +++++++++++++++++++++------
 src/test/regress/expected/jsonb.out | 30 +++++++++++++++++++++++++++++
 src/test/regress/sql/jsonb.sql      |  5 +++++
 3 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/src/backend/utils/adt/jsonb_gin.c b/src/backend/utils/adt/jsonb_gin.c
index f5dbd5589d3..96506661cb0 100644
--- a/src/backend/utils/adt/jsonb_gin.c
+++ b/src/backend/utils/adt/jsonb_gin.c
@@ -558,6 +558,7 @@ extract_jsp_path_expr_nodes(JsonPathGinContext *cxt, JsonPathGinPath path,
  * Extract an expression node from one of following jsonpath path expressions:
  *   EXISTS(jsp)    (when 'scalar' is NULL)
  *   jsp == scalar  (when 'scalar' is not NULL).
+ * Returns NULL if no usable filter expression can be extracted.
  *
  * The current path (@) is passed in 'path'.
  */
@@ -579,7 +580,10 @@ extract_jsp_path_expr(JsonPathGinContext *cxt, JsonPathGinPath path,
     return make_jsp_expr_node_args(JSP_GIN_AND, nodes);
 }

-/* Recursively extract nodes from the boolean jsonpath expression. */
+/*
+ * Recursively extract a GIN qual from the boolean jsonpath expression.
+ * Returns NULL if no usable indexqual can be extracted.
+ */
 static JsonPathGinNode *
 extract_jsp_bool_expr(JsonPathGinContext *cxt, JsonPathGinPath path,
                       JsonPathItem *jsp, bool not)
@@ -591,6 +595,8 @@ extract_jsp_bool_expr(JsonPathGinContext *cxt, JsonPathGinPath path,
         case jpiAnd:            /* expr && expr */
         case jpiOr:                /* expr || expr */
             {
+                /* We use De Morgan's laws to handle a passed-down NOT */
+                bool        is_and = (not ^ (jsp->type == jpiAnd));
                 JsonPathItem arg;
                 JsonPathGinNode *larg;
                 JsonPathGinNode *rarg;
@@ -604,13 +610,19 @@ extract_jsp_bool_expr(JsonPathGinContext *cxt, JsonPathGinPath path,

                 if (!larg || !rarg)
                 {
-                    if (jsp->type == jpiOr)
+                    /*
+                     * For OR, we have to fail if either arm is unsupported.
+                     * For AND, we can just ignore the unsupported arm and
+                     * apply the supported one, relying on indexscan qual
+                     * recheck to reject rows that don't match the other arm.
+                     */
+                    if (!is_and)
                         return NULL;

                     return larg ? larg : rarg;
                 }

-                type = not ^ (jsp->type == jpiAnd) ? JSP_GIN_AND : JSP_GIN_OR;
+                type = is_and ? JSP_GIN_AND : JSP_GIN_OR;

                 return make_jsp_expr_node_binary(type, larg, rarg);
             }
@@ -662,24 +674,28 @@ extract_jsp_bool_expr(JsonPathGinContext *cxt, JsonPathGinPath path,
                 JsonPathItem *scalar_item;
                 JsonbValue    scalar;

+                /* Can't support !(x == y) */
                 if (not)
                     return NULL;

                 jspGetLeftArg(jsp, &left_item);
                 jspGetRightArg(jsp, &right_item);

-                if (jspIsScalar(left_item.type))
+                /* To build an indexqual, one side must be a simple scalar */
+                if (jspIsScalar(left_item.type) &&
+                    !jspHasNext(&left_item))
                 {
                     scalar_item = &left_item;
                     path_item = &right_item;
                 }
-                else if (jspIsScalar(right_item.type))
+                else if (jspIsScalar(right_item.type) &&
+                         !jspHasNext(&right_item))
                 {
                     scalar_item = &right_item;
                     path_item = &left_item;
                 }
                 else
-                    return NULL;    /* at least one operand should be a scalar */
+                    return NULL;

                 switch (scalar_item->type)
                 {
diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index b1d2ce5f03a..82c31e2962b 100644
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -3053,6 +3053,12 @@ SELECT count(*) FROM testjsonb WHERE j @@ '$.age == 25.0';
      2
 (1 row)

+SELECT count(*) FROM testjsonb WHERE j @@ '$.age == "25".double()';
+ count
+-------
+     2
+(1 row)
+
 SELECT count(*) FROM testjsonb WHERE j @@ 'exists($)';
  count
 -------
@@ -3260,6 +3266,12 @@ SELECT count(*) FROM testjsonb WHERE j @@ '$.age == 25.0';
      2
 (1 row)

+SELECT count(*) FROM testjsonb WHERE j @@ '$.age == "25".double()';
+ count
+-------
+     2
+(1 row)
+
 SELECT count(*) FROM testjsonb WHERE j @@ '$.array[*] == "foo"';
  count
 -------
@@ -3272,6 +3284,12 @@ SELECT count(*) FROM testjsonb WHERE j @@ '$.array[*] == "bar"';
      3
 (1 row)

+SELECT count(*) FROM testjsonb WHERE j @@ '!(!($.bad == false) && $.coauthors == 98)';
+ count
+-------
+   850
+(1 row)
+
 SELECT count(*) FROM testjsonb WHERE j @@ 'exists($ ? (@.array[*] == "bar"))';
  count
 -------
@@ -3589,6 +3607,12 @@ SELECT count(*) FROM testjsonb WHERE j @@ '$.age == 25.0';
      2
 (1 row)

+SELECT count(*) FROM testjsonb WHERE j @@ '$.age == "25".double()';
+ count
+-------
+     2
+(1 row)
+
 SELECT count(*) FROM testjsonb WHERE j @@ '$.array[*] == "foo"';
  count
 -------
@@ -3601,6 +3625,12 @@ SELECT count(*) FROM testjsonb WHERE j @@ '$.array[*] == "bar"';
      3
 (1 row)

+SELECT count(*) FROM testjsonb WHERE j @@ '!(!($.bad == false) && $.coauthors == 98)';
+ count
+-------
+   850
+(1 row)
+
 SELECT count(*) FROM testjsonb WHERE j @@ 'exists($ ? (@.array[*] == "bar"))';
  count
 -------
diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql
index d4e8d7d8c9e..610ee28b97f 100644
--- a/src/test/regress/sql/jsonb.sql
+++ b/src/test/regress/sql/jsonb.sql
@@ -843,6 +843,7 @@ SELECT count(*) FROM testjsonb WHERE j @@ '"CC" == $.wait';
 SELECT count(*) FROM testjsonb WHERE j @@ '$.wait == "CC" && true == $.public';
 SELECT count(*) FROM testjsonb WHERE j @@ '$.age == 25';
 SELECT count(*) FROM testjsonb WHERE j @@ '$.age == 25.0';
+SELECT count(*) FROM testjsonb WHERE j @@ '$.age == "25".double()';
 SELECT count(*) FROM testjsonb WHERE j @@ 'exists($)';
 SELECT count(*) FROM testjsonb WHERE j @@ 'exists($.public)';
 SELECT count(*) FROM testjsonb WHERE j @@ 'exists($.bar)';
@@ -884,8 +885,10 @@ SELECT count(*) FROM testjsonb WHERE j @@ '"CC" == $.wait';
 SELECT count(*) FROM testjsonb WHERE j @@ '$.wait == "CC" && true == $.public';
 SELECT count(*) FROM testjsonb WHERE j @@ '$.age == 25';
 SELECT count(*) FROM testjsonb WHERE j @@ '$.age == 25.0';
+SELECT count(*) FROM testjsonb WHERE j @@ '$.age == "25".double()';
 SELECT count(*) FROM testjsonb WHERE j @@ '$.array[*] == "foo"';
 SELECT count(*) FROM testjsonb WHERE j @@ '$.array[*] == "bar"';
+SELECT count(*) FROM testjsonb WHERE j @@ '!(!($.bad == false) && $.coauthors == 98)';
 SELECT count(*) FROM testjsonb WHERE j @@ 'exists($ ? (@.array[*] == "bar"))';
 SELECT count(*) FROM testjsonb WHERE j @@ 'exists($.array ? (@[*] == "bar"))';
 SELECT count(*) FROM testjsonb WHERE j @@ 'exists($.array[*] ? (@ == "bar"))';
@@ -964,8 +967,10 @@ SELECT count(*) FROM testjsonb WHERE j @@ '"CC" == $.wait';
 SELECT count(*) FROM testjsonb WHERE j @@ '$.wait == "CC" && true == $.public';
 SELECT count(*) FROM testjsonb WHERE j @@ '$.age == 25';
 SELECT count(*) FROM testjsonb WHERE j @@ '$.age == 25.0';
+SELECT count(*) FROM testjsonb WHERE j @@ '$.age == "25".double()';
 SELECT count(*) FROM testjsonb WHERE j @@ '$.array[*] == "foo"';
 SELECT count(*) FROM testjsonb WHERE j @@ '$.array[*] == "bar"';
+SELECT count(*) FROM testjsonb WHERE j @@ '!(!($.bad == false) && $.coauthors == 98)';
 SELECT count(*) FROM testjsonb WHERE j @@ 'exists($ ? (@.array[*] == "bar"))';
 SELECT count(*) FROM testjsonb WHERE j @@ 'exists($.array ? (@[*] == "bar"))';
 SELECT count(*) FROM testjsonb WHERE j @@ 'exists($.array[*] ? (@ == "bar"))';
--
2.52.0