Re: pruning disabled for array, enum, record, range type partition keys

Thanks for the review.

On 2018/04/18 7:11, Alvaro Herrera wrote:
> Amit Langote wrote:
>
>> Ah, I think I got it after staring at the (btree) index code for a bit.
>>
>> What pruning code got wrong is that it's comparing the expression type
>> (type of the constant arg that will be compared with partition bound
>> datums when pruning) with the partopcintype to determine if we should look
>> up the cross-type comparison/hashing procedure, whereas what the latter
>> should be compare with is the clause operator's oprighttype. ISTM, if
>> op_in_opfamily() passed for the operator, that's the correct thing to do.
>
> I wonder why you left out the hash partitioning case? I don't really
> know that this is correct, but here's a delta patch as demonstration.

@@ -1546,8 +1546,8 @@ match_clause_to_partition_key(RelOptInfo *rel,
case PARTITION_STRATEGY_HASH:
cmpfn = get_opfamily_proc(part_scheme->partopfamily[partkeyidx],
- op_righttype, op_righttype,
- HASHEXTENDED_PROC);
+ part_scheme->partopcintype[partkeyidx],
+ op_righttype, HASHEXTENDED_PROC);

This change is not quite right, because it disables pruning. The above
returns InvalidOid as there are no hash AM procedures (in pg_amproc) whose
lefttype and righttype don't match.

select count(*)
from pg_amproc
where amprocfamily in
(select opf.oid
from pg_opfamily opf join pg_am am on (opf.opfmethod = am.oid)
where amname = 'hash')
and amproclefttype <> amprocrighttype;

count
-------
0
(1 row)

So, the original coding passes op_righttype for both lefttype and righttype.

Consider the following example:

create table h(a int) partition by hash (a);
create table h1 partition of foo for values with (modulus 2, remainder 0);
create table h2 partition of foo for values with (modulus 2, remainder 1);

insert into h values (1::bigint);
select tableoid::regclass, * from h;
tableoid | a
----------+---
h1 | 1
(1 row)

-- without the delta patch
explain select * from h where a = 1::bigint;
QUERY PLAN
------------------------------------------------------------
Append (cost=0.00..41.94 rows=13 width=4)
-> Seq Scan on h1 (cost=0.00..41.88 rows=13 width=4)
Filter: (a = '1'::bigint)
(3 rows)

-- with
explain select * from h where a = 1::bigint;
QUERY PLAN
------------------------------------------------------------
Append (cost=0.00..83.88 rows=26 width=4)
-> Seq Scan on h1 (cost=0.00..41.88 rows=13 width=4)
Filter: (a = '1'::bigint)
-> Seq Scan on h2 (cost=0.00..41.88 rows=13 width=4)
Filter: (a = '1'::bigint)
(5 rows)

> (v3 is your patch, I think the only change is I renamed the tables used
> in the test)

Thanks, that seems like a good idea.

Here's v4 with parts of your delta patch merged. I've also updated
comments in match_clause_to_partition_key() to describe the rationale of
support function look up in a bit more detail.

Regards,
Amit