public inbox for [email protected]  
help / color / mirror / Atom feed
From: Andrei Lepikhov <[email protected]>
To: Alexander Korotkov <[email protected]>
Cc: Alexander Lakhin <[email protected]>
Cc: [email protected]
Cc: Pavel Borisov <[email protected]>
Cc: a.rybakina <[email protected]>
Subject: Re: POC, WIP: OR-clause support for indexes
Date: Mon, 24 Mar 2025 11:10:00 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <CALT9ZEE94JN01za2o872foQErgyLHaX7OnOtoh70QbmXP7=S2A@mail.gmail.com>
References: <[email protected]>
	<CAEudQApBzH9JNkDpXARrjFG0WGM2PcU02f6Ga9sLVS2H6z9YAQ@mail.gmail.com>
	<[email protected]>
	<CAPpHfdvM4E-wPCRbYkr9eGH2sSxgOYKZ1iOUyjKYqTEyurFRLQ@mail.gmail.com>
	<[email protected]>
	<CAPpHfdtgcye9ELaLHw4ZOUxWQ9t1P=oBDJX8KwwOHqasESdXQQ@mail.gmail.com>
	<CAPpHfdv+jtNwofg-p5z86jLYZUTt6tR17Wy00ta0dL=wHQN3ZA@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CAPpHfdvehQbCpQG9zYr739y2nXQkE=FYsat5VE-pDoB4RDPfBw@mail.gmail.com>
	<[email protected]>
	<CAPpHfdtPz+J-n6=qAMBqx1p88Ut_cbS9JRbL+EKRpWY04tGcRg@mail.gmail.com>
	<[email protected]>
	<CAPpHfdumgFuUihGqrmDVR5FnzOLsAah-ZdZ269FgsUrOeNU6rw@mail.gmail.com>
	<[email protected]>
	<CALT9ZEE94JN01za2o872foQErgyLHaX7OnOtoh70QbmXP7=S2A@mail.gmail.com>

Hi,

Playing with the feature, I found a slightly irritating permutation - 
even if this code doesn't group any clauses, it may permute positions of 
the quals. See:

DROP TABLE IF EXISTS main_tbl;
CREATE TABLE main_tbl(id bigint, hundred int, thousand int);
CREATE INDEX mt_hundred_ix ON main_tbl(hundred);
CREATE INDEX mt_thousand_ix ON main_tbl(thousand);
VACUUM (ANALYZE) main_tbl;

SET enable_seqscan = off;
EXPLAIN (COSTS OFF)
SELECT m.id, m.hundred, m.thousand
FROM main_tbl m WHERE (m.hundred < 2 OR m.thousand < 3);

  Bitmap Heap Scan on public.main_tbl m
    Output: id, hundred, thousand
    Recheck Cond: ((m.thousand < 3) OR (m.hundred < 2))
    ->  BitmapOr
          ->  Bitmap Index Scan on mt_thousand_ix
                Index Cond: (m.thousand < 3)
          ->  Bitmap Index Scan on mt_hundred_ix
                Index Cond: (m.hundred < 2)

Conditions on the columns "thousand" and "hundred" changed their places 
according to the initial positions defined in the user's SQL.
It isn't okay. I see that users often use the trick of "OR order" to 
avoid unnecessary calculations - most frequently, Subplan evaluations. 
So, it makes sense to fix.
In the attachment, I have included a quick fix for this issue. Although 
many tests returned to their initial (pre-18) state, I added some tests 
specifically related to this issue to make it clearer.

-- 
regards, Andrei Lepikhov

Attachments:

  [text/x-patch] clause-permutation-fix.diff (9.1K, ../[email protected]/2-clause-permutation-fix.diff)
  download | inline diff:
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index a43ca16d68..7d8ef0c90f 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -1254,6 +1254,7 @@ group_similar_or_args(PlannerInfo *root, RelOptInfo *rel, RestrictInfo *rinfo)
 	ListCell   *lc;
 	ListCell   *lc2;
 	List	   *orargs;
+	bool		grouping_happened = false;
 	List	   *result = NIL;
 	Index		relid = rel->relid;
 
@@ -1465,13 +1466,18 @@ group_similar_or_args(PlannerInfo *root, RelOptInfo *rel, RestrictInfo *rinfo)
 												   rinfo->incompatible_relids,
 												   rinfo->outer_relids);
 				result = lappend(result, subrinfo);
+				grouping_happened = true;
 			}
 
 			group_start = i;
 		}
 	}
 	pfree(matches);
-	return result;
+
+	if (grouping_happened)
+		return result;
+	else
+		return orargs;
 }
 
 /*
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index bd5f002cf2..c6f84bda64 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -3248,6 +3248,37 @@ SELECT  b.relname,
 (2 rows)
 
 DROP TABLE concur_temp_tab_1, concur_temp_tab_2, reindex_temp_before;
+-- No OR-clause groupings should happen - no clause permutations in
+-- the filtering conditions we should see in the EXPLAIN.
+EXPLAIN (COSTS OFF)
+SELECT * FROM tenk1 WHERE unique1 < 1 OR hundred < 2;
+                    QUERY PLAN                    
+--------------------------------------------------
+ Bitmap Heap Scan on tenk1
+   Recheck Cond: ((unique1 < 1) OR (hundred < 2))
+   ->  BitmapOr
+         ->  Bitmap Index Scan on tenk1_unique1
+               Index Cond: (unique1 < 1)
+         ->  Bitmap Index Scan on tenk1_hundred
+               Index Cond: (hundred < 2)
+(7 rows)
+
+-- OR clauses on the 'unique' column is grouped. So, clause permutation happened
+-- We see it in the 'Recheck Cond' and order of BitmapOr subpaths: index scan on
+-- the 'hundred' column occupies the first position.
+EXPLAIN (COSTS OFF)
+SELECT * FROM tenk1 WHERE unique1 < 1 OR unique1 < 3 OR hundred < 2;
+                             QUERY PLAN                              
+---------------------------------------------------------------------
+ Bitmap Heap Scan on tenk1
+   Recheck Cond: ((hundred < 2) OR ((unique1 < 1) OR (unique1 < 3)))
+   ->  BitmapOr
+         ->  Bitmap Index Scan on tenk1_hundred
+               Index Cond: (hundred < 2)
+         ->  Bitmap Index Scan on tenk1_unique1
+               Index Cond: (unique1 < ANY ('{1,3}'::integer[]))
+(7 rows)
+
 -- Check bitmap scan can consider similar OR arguments separately without
 -- grouping them into SAOP.
 CREATE TABLE bitmap_split_or (a int NOT NULL, b int NOT NULL, c int NOT NULL);
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index a57bb18c24..a950153d76 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -4333,20 +4333,20 @@ select * from tenk1 a join tenk1 b on
  Nested Loop
    Join Filter: (((a.unique1 = 1) AND (b.unique1 = 2)) OR ((a.unique2 = 3) AND (b.hundred = 4)))
    ->  Bitmap Heap Scan on tenk1 b
-         Recheck Cond: ((hundred = 4) OR (unique1 = 2))
+         Recheck Cond: ((unique1 = 2) OR (hundred = 4))
          ->  BitmapOr
-               ->  Bitmap Index Scan on tenk1_hundred
-                     Index Cond: (hundred = 4)
                ->  Bitmap Index Scan on tenk1_unique1
                      Index Cond: (unique1 = 2)
+               ->  Bitmap Index Scan on tenk1_hundred
+                     Index Cond: (hundred = 4)
    ->  Materialize
          ->  Bitmap Heap Scan on tenk1 a
-               Recheck Cond: ((unique2 = 3) OR (unique1 = 1))
+               Recheck Cond: ((unique1 = 1) OR (unique2 = 3))
                ->  BitmapOr
-                     ->  Bitmap Index Scan on tenk1_unique2
-                           Index Cond: (unique2 = 3)
                      ->  Bitmap Index Scan on tenk1_unique1
                            Index Cond: (unique1 = 1)
+                     ->  Bitmap Index Scan on tenk1_unique2
+                           Index Cond: (unique2 = 3)
 (17 rows)
 
 explain (costs off)
@@ -4360,12 +4360,12 @@ select * from tenk1 a join tenk1 b on
          Filter: ((unique1 = 2) OR (ten = 4))
    ->  Materialize
          ->  Bitmap Heap Scan on tenk1 a
-               Recheck Cond: ((unique2 = 3) OR (unique1 = 1))
+               Recheck Cond: ((unique1 = 1) OR (unique2 = 3))
                ->  BitmapOr
-                     ->  Bitmap Index Scan on tenk1_unique2
-                           Index Cond: (unique2 = 3)
                      ->  Bitmap Index Scan on tenk1_unique1
                            Index Cond: (unique1 = 1)
+                     ->  Bitmap Index Scan on tenk1_unique2
+                           Index Cond: (unique2 = 3)
 (12 rows)
 
 explain (costs off)
@@ -4377,12 +4377,12 @@ select * from tenk1 a join tenk1 b on
  Nested Loop
    Join Filter: (((a.unique1 = 1) AND (b.unique1 = 2)) OR (((a.unique2 = 3) OR (a.unique2 = 7)) AND (b.hundred = 4)))
    ->  Bitmap Heap Scan on tenk1 b
-         Recheck Cond: ((hundred = 4) OR (unique1 = 2))
+         Recheck Cond: ((unique1 = 2) OR (hundred = 4))
          ->  BitmapOr
-               ->  Bitmap Index Scan on tenk1_hundred
-                     Index Cond: (hundred = 4)
                ->  Bitmap Index Scan on tenk1_unique1
                      Index Cond: (unique1 = 2)
+               ->  Bitmap Index Scan on tenk1_hundred
+                     Index Cond: (hundred = 4)
    ->  Materialize
          ->  Bitmap Heap Scan on tenk1 a
                Recheck Cond: (((unique2 = 3) OR (unique2 = 7)) OR (unique1 = 1))
@@ -4403,12 +4403,12 @@ select * from tenk1 a join tenk1 b on
  Nested Loop
    Join Filter: (((a.unique1 = 1) AND (b.unique1 = 2)) OR (((a.unique2 = 3) OR (a.unique2 = 7)) AND (b.hundred = 4)))
    ->  Bitmap Heap Scan on tenk1 b
-         Recheck Cond: ((hundred = 4) OR (unique1 = 2))
+         Recheck Cond: ((unique1 = 2) OR (hundred = 4))
          ->  BitmapOr
-               ->  Bitmap Index Scan on tenk1_hundred
-                     Index Cond: (hundred = 4)
                ->  Bitmap Index Scan on tenk1_unique1
                      Index Cond: (unique1 = 2)
+               ->  Bitmap Index Scan on tenk1_hundred
+                     Index Cond: (hundred = 4)
    ->  Materialize
          ->  Bitmap Heap Scan on tenk1 a
                Recheck Cond: (((unique2 = 3) OR (unique2 = 7)) OR (unique1 = 1))
diff --git a/src/test/regress/expected/partition_join.out b/src/test/regress/expected/partition_join.out
index 938cedd79a..6101c8c7cf 100644
--- a/src/test/regress/expected/partition_join.out
+++ b/src/test/regress/expected/partition_join.out
@@ -2568,24 +2568,24 @@ where not exists (select 1 from prtx2
          ->  Seq Scan on prtx1_1
                Filter: ((a < 20) AND (c = 91))
          ->  Bitmap Heap Scan on prtx2_1
-               Recheck Cond: ((c = 99) OR (b = (prtx1_1.b + 1)))
+               Recheck Cond: ((b = (prtx1_1.b + 1)) OR (c = 99))
                Filter: (a = prtx1_1.a)
                ->  BitmapOr
-                     ->  Bitmap Index Scan on prtx2_1_c_idx
-                           Index Cond: (c = 99)
                      ->  Bitmap Index Scan on prtx2_1_b_idx
                            Index Cond: (b = (prtx1_1.b + 1))
+                     ->  Bitmap Index Scan on prtx2_1_c_idx
+                           Index Cond: (c = 99)
    ->  Nested Loop Anti Join
          ->  Seq Scan on prtx1_2
                Filter: ((a < 20) AND (c = 91))
          ->  Bitmap Heap Scan on prtx2_2
-               Recheck Cond: ((c = 99) OR (b = (prtx1_2.b + 1)))
+               Recheck Cond: ((b = (prtx1_2.b + 1)) OR (c = 99))
                Filter: (a = prtx1_2.a)
                ->  BitmapOr
-                     ->  Bitmap Index Scan on prtx2_2_c_idx
-                           Index Cond: (c = 99)
                      ->  Bitmap Index Scan on prtx2_2_b_idx
                            Index Cond: (b = (prtx1_2.b + 1))
+                     ->  Bitmap Index Scan on prtx2_2_c_idx
+                           Index Cond: (c = 99)
 (23 rows)
 
 select * from prtx1
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index be570da08a..51e030294e 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -1355,6 +1355,17 @@ SELECT  b.relname,
   ORDER BY 1;
 DROP TABLE concur_temp_tab_1, concur_temp_tab_2, reindex_temp_before;
 
+-- No OR-clause groupings should happen - no clause permutations in
+-- the filtering conditions we should see in the EXPLAIN.
+EXPLAIN (COSTS OFF)
+SELECT * FROM tenk1 WHERE unique1 < 1 OR hundred < 2;
+
+-- OR clauses on the 'unique' column is grouped. So, clause permutation happened
+-- We see it in the 'Recheck Cond' and order of BitmapOr subpaths: index scan on
+-- the 'hundred' column occupies the first position.
+EXPLAIN (COSTS OFF)
+SELECT * FROM tenk1 WHERE unique1 < 1 OR unique1 < 3 OR hundred < 2;
+
 -- Check bitmap scan can consider similar OR arguments separately without
 -- grouping them into SAOP.
 CREATE TABLE bitmap_split_or (a int NOT NULL, b int NOT NULL, c int NOT NULL);


view thread (57+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: POC, WIP: OR-clause support for indexes
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox