public inbox for [email protected]  
help / color / mirror / Atom feed
From: Andreas Karlsson <[email protected]>
To: Teodor Sigaev <[email protected]>
To: Tomas Vondra <[email protected]>
Cc: David Rowley <[email protected]>
Cc: Pgsql Hackers <[email protected]>
Subject: Re: POC, WIP: OR-clause support for indexes
Date: Sat, 19 Mar 2016 00:46:26 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<CAKJS1f_NuyEbHfUeSqBFw0G7jmLM5RC5PKRqcAxO0vnJWCgRcg@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
List-Unsubscribe: <mailto:[email protected]?body=unsub%20pgsql-hackers>

I gave this patch a quick spin and noticed a strange query plan.

CREATE TABLE test (a int, b int, c int);
CREATE INDEX ON test USING gin (a, b, c);
INSERT INTO test SELECT i % 7, i % 9, i % 11 FROM generate_series(1, 
1000000) i;
EXPLAIN ANALYZE SELECT * FROM test WHERE (a = 3 OR b = 5) AND c = 2;

                                                             QUERY PLAN 

----------------------------------------------------------------------------------------------------------------------------------
  Bitmap Heap Scan on test  (cost=829.45..4892.10 rows=21819 width=12) 
(actual time=66.494..76.234 rows=21645 loops=1)
    Recheck Cond: ((((a = 3) AND (c = 2)) OR ((b = 5) AND (c = 2))) AND 
(c = 2))
    Heap Blocks: exact=5406
    ->  Bitmap Index Scan on test_a_b_c_idx  (cost=0.00..824.00 
rows=2100 width=0) (actual time=65.272..65.272 rows=21645 loops=1)
          Index Cond: ((((a = 3) AND (c = 2)) OR ((b = 5) AND (c = 2))) 
AND (c = 2))
  Planning time: 0.200 ms
  Execution time: 77.206 ms
(7 rows)

Shouldn't the index condition just be "((a = 3) AND (c = 2)) OR ((b = 5) 
AND (c = 2))"?

Also when applying and reading the patch I noticed some minor 
issues/nitpick.

- I get whitespace warnings from git apply when I apply the patches.
- You have any insconstent style for casts: I think "(Node*)clause" 
should be "(Node *) clause".
- Same with pointers. "List* quals" should be "List *quals"
- I am personally not a fan of seeing the "isorderby == false && 
index->rd_amroutine->amcanorclause" clause twice. Feels like a risk for 
diverging code paths. But it could be that there is no clean alternative.

Andreas


-- 
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



view thread (29+ 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]
  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