Message-ID: <670d1dc2-a280-4cf6-bc29-e2eafb107081@gmail.com>
Date: Fri, 23 Aug 2024 12:23:49 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: POC, WIP: OR-clause support for indexes
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: Alena Rybakina <a.rybakina@postgrespro.ru>,
 Nikolay Shaplov <dhyan@nataraj.su>, pgsql-hackers@lists.postgresql.org,
 jian he <jian.universality@gmail.com>, Robert Haas <robertmhaas@gmail.com>,
 pgsql-hackers@postgresql.org, Peter Geoghegan <pg@bowt.ie>,
 Marcos Pegoraro <marcos@f10.com.br>, teodor@sigaev.ru,
 Peter Eisentraut <peter@eisentraut.org>, Ranier Vilela <ranier.vf@gmail.com>
References: <567ED6CA.2040504@sigaev.ru> <8969055.VV5PYv0bhD@thinkpad-pgpro>
 <38289f6c-b8c0-4713-8fb5-703fe771872a@postgrespro.ru>
 <CAPpHfdsrtAB=0B-qYuQMPgyyy=-CycFJfzTWO_30nx27vXc3dA@mail.gmail.com>
 <759292d5-cb51-4b12-89fa-576c1d9b374d@postgrespro.ru>
 <CAPpHfdvUEjkuFNUbg7GuLkXotwvpDgBYz4S8YcE=LRf9aBQh-Q@mail.gmail.com>
 <531fc0ab-371e-4235-97e3-dd2d077b6995@postgrespro.ru>
 <CAPpHfdu5iQOjF93vGbjidsQkhHvY2NSm29duENYH_cbhC6x+Mg@mail.gmail.com>
 <CAPpHfdv9GrXfCOdPsMszBrORig+nW2+JBuMD-np_keVuKJ7bdQ@mail.gmail.com>
 <cf2c669f-93ec-4e35-a521-cecac67def73@postgrespro.ru>
 <CAPpHfduTM2W3kmiN1=5fvgcu6yA5=ghq82OkvLvZC8Mh9PpEuA@mail.gmail.com>
 <c3c490e2-1f04-420b-bd5c-f4840d6a7198@postgrespro.ru>
 <e657e536-f10b-4131-a3f2-667c73c86588@postgrespro.ru>
 <CAPpHfduFJHTNGCTFWt4GFs-E6Ts8xS5mCsKOYKBsZAmoOr8ZSA@mail.gmail.com>
 <65bfdcb6-0f0c-4d4c-a721-cb38a0ba91c7@postgrespro.ru>
 <CAPpHfdtR3cBKCSj_7dd7EDF_pa-rUUQBx3-guK6kZZ=_hSeMfg@mail.gmail.com>
Content-Language: en-US
From: Andrei Lepikhov <lepihov@gmail.com>
In-Reply-To: <CAPpHfdtR3cBKCSj_7dd7EDF_pa-rUUQBx3-guK6kZZ=_hSeMfg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: <https://www.postgresql.org/message-id/670d1dc2-a280-4cf6-bc29-e2eafb107081%40gmail.com>
Precedence: bulk

On 21/8/2024 16:52, Alexander Korotkov wrote:
>> /* Only operator clauses scan match */
>> Should it be:
>> /* Only operator clauses can match */
>> ?
> 
> Corrected, thanks.
I found one more: /* Only operator clauses scan match  */ - in the 
second patch.
Also I propose:
- “might match to the index as whole” -> “might match the index as a whole“
- Group similar OR-arguments intro dedicated RestrictInfos -> ‘into’
> 
>> The second one:
>> When creating IndexClause, we assign the original and derived clauses to
>> the new, containing transformed array. But logically, we should set the
>> clause with a list of ORs as the original. Why did you do so?
> 
> I actually didn't notice that.  Corrected to set the OR clause as the
> original.  That change turned recheck to use original OR clauses,
> probably better this way.  Also, that change spotted misuse of
> RestrictInfo.clause and RestrictInfo.orclause in the second patch.
> Corrected this too.
New findings:
=============

1)
if (list_length(clause->args) != 2)
	return NULL;
I guess, above we can 'continue' the process.

2) Calling the match_index_to_operand in three nested cycles you could 
break the search on first successful match, couldn't it? At least, the 
comment "just stop with first matching index key" say so.

3) I finally found the limit of this feature: the case of two partial 
indexes on the same column. Look at the example below:

SET enable_indexscan = 'off';
SET enable_seqscan = 'off';
DROP TABLE IF EXISTS test CASCADE;
CREATE TABLE test (x int);
INSERT INTO test (x) SELECT * FROM generate_series(1,100);
CREATE INDEX ON test (x) WHERE x < 80;
CREATE INDEX ON test (x) WHERE x > 80;
VACUUM ANALYZE test;
EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF)
SELECT * FROM test WHERE x=1 OR x = 79;
EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF)
SELECT * FROM test WHERE x=91 OR x = 81;
EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF)
SELECT * FROM test WHERE x=1 OR x = 81 OR x = 83;

The last query doesn't group clauses into two indexes. The reason is in 
match_index_to_operand which classifies all 'x=' to one class. I'm not 
sure because of overhead, but it may be resolved by using 
predicate_implied_by to partial indexes.

-- 
regards, Andrei Lepikhov