From d987ad61d3b1cc3881d85352761607f496e45751 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 13 Oct 2026 16:33:35 -0400 Subject: [PATCH] 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 a4d7d24509a..4ad8dcd4682 100644 --- a/src/backend/utils/adt/jsonb_gin.c +++ b/src/backend/utils/adt/jsonb_gin.c @@ -557,6 +557,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'. */ @@ -578,7 +579,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) @@ -590,6 +594,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; @@ -603,13 +609,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); } @@ -661,24 +673,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 3c5be773a1a..f6d02fa3553 100644 --- a/src/test/regress/expected/jsonb.out +++ b/src/test/regress/expected/jsonb.out @@ -2862,6 +2862,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 ------- @@ -3068,6 +3074,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 ------- @@ -3080,6 +3092,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 ------- @@ -3397,6 +3415,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 ------- @@ -3409,6 +3433,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 040e1bac212..59a2ef61d77 100644 --- a/src/test/regress/sql/jsonb.sql +++ b/src/test/regress/sql/jsonb.sql @@ -782,6 +782,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)'; @@ -822,8 +823,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"))'; @@ -902,8 +905,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"))';