From 327a2ad9221f146e9fe323e6f121db980a46ad8b Mon Sep 17 00:00:00 2001 From: Tom Lane 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 Author: Tom Lane 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