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

2018-04-19 Thread Alvaro Herrera
Amit Langote wrote:

> On 2018/04/19 6:45, Alvaro Herrera wrote:

> > Please give this version another look.  I also rewrote a couple of
> > comments.
> 
> Thanks, your rewritten version looks much better.

Thanks!  Pushed now.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



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

2018-04-19 Thread Amit Langote
Hi.

On 2018/04/19 6:45, Alvaro Herrera wrote:
> Amit Langote wrote:
>> On Thu, Apr 19, 2018 at 12:01 AM, Alvaro Herrera
>>  wrote:
> 
>>> Makes sense.  Still, I was expecting that pruning of hash partitioning
>>> would also work for pseudotypes, yet it doesn't.
>>
>> It does?
> 
> Aha, so it does.
> 
> While staring at this new code, I was confused as to why we didn't use
> the commutator if the code above had determined one.  I was unable to
> cause a test to fail, so I put that thought aside.

Oops, you're right.  Shouldn't have ignored the commutator.

> Some time later, after restructuring the code in a way that seemed to
> make more sense to me (and saving one get_op_opfamily_properties call
> for the case of the not-equals operator), I realized that with the new
> code we can store the opstrategy in the PartClause instead of leaving it
> as Invalid and look it up again later, so I did that.  And lo and
> behold, the tests that used commutators started failing!  So I fixed
> that one in the obvious way, and the tests work fully again.
> 
> Please give this version another look.  I also rewrote a couple of
> comments.

Thanks, your rewritten version looks much better.

> I now wonder if there's anything else that equivclass.c or indxpath.c
> can teach us on this topic.

I have referenced indxpath.c number of times when writing this code (for
example, match_clause_to_indexcol), but never equivclass.c.

Thanks,
Amit




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

2018-04-18 Thread Tom Lane
Alvaro Herrera  writes:
> I now wonder if there's anything else that equivclass.c or indxpath.c
> can teach us on this topic.

I've been meaning to review this but have been a bit distracted.
Will try to look tomorrow.

regards, tom lane



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

2018-04-18 Thread Alvaro Herrera
Amit Langote wrote:
> On Thu, Apr 19, 2018 at 12:01 AM, Alvaro Herrera
>  wrote:

> > Makes sense.  Still, I was expecting that pruning of hash partitioning
> > would also work for pseudotypes, yet it doesn't.
> 
> It does?

Aha, so it does.

While staring at this new code, I was confused as to why we didn't use
the commutator if the code above had determined one.  I was unable to
cause a test to fail, so I put that thought aside.

Some time later, after restructuring the code in a way that seemed to
make more sense to me (and saving one get_op_opfamily_properties call
for the case of the not-equals operator), I realized that with the new
code we can store the opstrategy in the PartClause instead of leaving it
as Invalid and look it up again later, so I did that.  And lo and
behold, the tests that used commutators started failing!  So I fixed
that one in the obvious way, and the tests work fully again.

Please give this version another look.  I also rewrote a couple of
comments.

I now wonder if there's anything else that equivclass.c or indxpath.c
can teach us on this topic.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 340b0b3a97af0fa07562bc4434a4908402c3efbd Mon Sep 17 00:00:00 2001
From: Alvaro Herrera 
Date: Wed, 18 Apr 2018 18:18:51 -0300
Subject: [PATCH v5] Fix pruning code to determine comparison function
 correctly

It's unreliable to determine one using the constant expression's
type directly (for example, it doesn't work correctly when the
expression contains an array, enum, etc.).  Instead, use righttype
of the operator, the one that supposedly passed the op_in_opfamily
test using the partitioning opfamily.
---
 src/backend/partitioning/partprune.c  | 101 +++
 src/test/regress/expected/partition_prune.out | 138 ++
 src/test/regress/sql/partition_prune.sql  |  53 ++
 3 files changed, 250 insertions(+), 42 deletions(-)

diff --git a/src/backend/partitioning/partprune.c 
b/src/backend/partitioning/partprune.c
index 7666c6c412..b8a006e774 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -1426,10 +1426,12 @@ match_clause_to_partition_key(RelOptInfo *rel,
OpExpr *opclause = (OpExpr *) clause;
Expr   *leftop,
   *rightop;
-   Oid commutator = InvalidOid,
+   Oid op_lefttype,
+   op_righttype,
+   commutator = InvalidOid,
negator = InvalidOid;
Oid cmpfn;
-   Oid exprtype;
+   int op_strategy;
boolis_opne_listp = false;
PartClauseInfo *partclause;
 
@@ -1483,33 +1485,39 @@ match_clause_to_partition_key(RelOptInfo *rel,
return PARTCLAUSE_UNSUPPORTED;
 
/*
-* Normally we only bother with operators that are listed as 
being
-* part of the partitioning operator family.  But we make an 
exception
-* in one case -- operators named '<>' are not listed in any 
operator
-* family whatsoever, in which case, we try to perform partition
-* pruning with it only if list partitioning is in use.
+* Determine the input types of the operator we're considering.
+*
+* Normally we only care about operators that are listed as 
being part
+* of the partitioning operator family.  But there is one 
exception:
+* the not-equals operators are not listed in any operator 
family
+* whatsoever, but we can find their negator in the opfamily 
and it's
+* an equality op.  We can use one of those if we find it, but 
they
+* are only useful for list partitioning.
 */
-   if (!op_in_opfamily(opclause->opno, partopfamily))
+   if (op_in_opfamily(opclause->opno, partopfamily))
{
+   Oid oper;
+
+   oper = OidIsValid(commutator) ? commutator : 
opclause->opno;
+   get_op_opfamily_properties(oper, partopfamily, false,
+  
_strategy, _lefttype,
+  
_righttype);
+   }
+   else
+   {
+   /* Not good unless list partitioning */
if (part_scheme->strategy != PARTITION_STRATEGY_LIST)
return 

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

2018-04-18 Thread Amit Langote
On Thu, Apr 19, 2018 at 12:01 AM, Alvaro Herrera
 wrote:
> Amit Langote wrote:
>
>> On 2018/04/18 7:11, Alvaro Herrera wrote:
>>
>> @@ -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.
>
> Makes sense.  Still, I was expecting that pruning of hash partitioning
> would also work for pseudotypes, yet it doesn't.

It does?

+-- array type hash partition key
+create table pph_arrpart (a int[]) partition by hash (a);
+create table pph_arrpart1 partition of pph_arrpart for values with
(modulus 2, remainder 0);
+create table pph_arrpart2 partition of pph_arrpart for values with
(modulus 2, remainder 1);
+insert into pph_arrpart values ('{1}'), ('{1, 2}'), ('{4, 5}');
+select tableoid::regclass, * from pph_arrpart order by 1;
+   tableoid   |   a
+--+---
+ pph_arrpart1 | {1,2}
+ pph_arrpart1 | {4,5}
+ pph_arrpart2 | {1}
+(3 rows)
+
+explain (costs off) select * from pph_arrpart where a = '{1}';
+   QUERY PLAN
+
+ Append
+   ->  Seq Scan on pph_arrpart2
+ Filter: (a = '{1}'::integer[])
+(3 rows)
+
+explain (costs off) select * from pph_arrpart where a = '{1, 2}';
+QUERY PLAN
+--
+ Append
+   ->  Seq Scan on pph_arrpart1
+ Filter: (a = '{1,2}'::integer[])
+(3 rows)

Thanks,
Amit



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

2018-04-18 Thread Alvaro Herrera
Amit Langote wrote:

> On 2018/04/18 7:11, Alvaro Herrera wrote:
> 
> @@ -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.

Makes sense.  Still, I was expecting that pruning of hash partitioning
would also work for pseudotypes, yet it doesn't.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



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

2018-04-17 Thread Amit Langote
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
>From d6652eb932a5e1cf6b63fa1dd09e4dd752267dbc Mon Sep 17 00:00:00 2001
From: amit 
Date: Wed, 18 Apr 2018 10:40:14 +0900
Subject: [PATCH v4] Fix pruning code to determine comparison function
 correctly

It's unreliable to determine one using the constant expression's
type directly (for example, it doesn't work correctly when the
expression contains an array, enum, etc.).  Instead, use righttype
of the operator, the one that supposedly passed the op_in_opfamily
test using the partitioning opfamily.
---
 src/backend/partitioning/partprune.c  |  45 +++--
 src/test/regress/expected/partition_prune.out | 138 ++
 src/test/regress/sql/partition_prune.sql  |  53 ++
 3 files changed, 227 insertions(+), 9 deletions(-)

diff --git a/src/backend/partitioning/partprune.c 
b/src/backend/partitioning/partprune.c
index 7666c6c412..6ab1066dea 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -1426,10 +1426,12 @@ match_clause_to_partition_key(RelOptInfo *rel,
OpExpr *opclause = (OpExpr *) clause;
Expr   *leftop,
   *rightop;
-   Oid commutator = InvalidOid,
+   Oid op_lefttype,
+   op_righttype,
+   commutator = InvalidOid,
negator = InvalidOid;
Oid cmpfn;
-   Oid exprtype;
+   int op_strategy;
boolis_opne_listp = false;
PartClauseInfo *partclause;
 
@@ -1517,24 +1519,51 @@ match_clause_to_partition_key(RelOptInfo *rel,
return PARTCLAUSE_UNSUPPORTED;
}
 
-  

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

2018-04-17 Thread Alvaro Herrera
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.

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

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/partitioning/partprune.c 
b/src/backend/partitioning/partprune.c
index 2655d2caa2..711e811efc 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -1523,8 +1523,8 @@ match_clause_to_partition_key(RelOptInfo *rel,
 * Check if we're going to need a cross-type comparison 
function to
 * use during pruning.
 */
-   get_op_opfamily_properties(OidIsValid(negator)
-   ? 
negator : opclause->opno,
+   get_op_opfamily_properties(OidIsValid(negator) ?
+  negator : 
opclause->opno,
   
partopfamily, false,
   
_strategy, _lefttype, _righttype);
/* Use the cached one if no cross-type comparison. */
@@ -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);
break;
 
default:
diff --git a/src/test/regress/expected/partition_prune.out 
b/src/test/regress/expected/partition_prune.out
index 697a3620a7..b0010d8ebb 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -2633,6 +2633,37 @@ explain (costs off) select * from pp_arrpart where a in 
('{4, 5}', '{1}');
 (5 rows)
 
 drop table pp_arrpart;
+-- array type hash partition key
+create table pph_arrpart (a int[]) partition by hash (a);
+create table pph_arrpart1 partition of pph_arrpart for values with (modulus 2, 
remainder 0);
+create table pph_arrpart2 partition of pph_arrpart for values with (modulus 2, 
remainder 1);
+explain (costs off) select * from pph_arrpart where a = '{1}';
+   QUERY PLAN   
+
+ Append
+   ->  Seq Scan on pph_arrpart2
+ Filter: (a = '{1}'::integer[])
+(3 rows)
+
+explain (costs off) select * from pph_arrpart where a = '{1, 2}';
+QUERY PLAN
+--
+ Append
+   ->  Seq Scan on pph_arrpart1
+ Filter: (a = '{1,2}'::integer[])
+(3 rows)
+
+explain (costs off) select * from pph_arrpart where a in ('{4, 5}', '{1}');
+  QUERY PLAN  
+--
+ Append
+   ->  Seq Scan on pph_arrpart1
+ Filter: ((a = '{4,5}'::integer[]) OR (a = '{1}'::integer[]))
+   ->  Seq Scan on pph_arrpart2
+ Filter: ((a = '{4,5}'::integer[]) OR (a = '{1}'::integer[]))
+(5 rows)
+
+drop table pph_arrpart;
 -- enum type list partition key
 create type pp_colors as enum ('green', 'blue', 'black');
 create table pp_enumpart (a pp_colors) partition by list (a);
diff --git a/src/test/regress/sql/partition_prune.sql 
b/src/test/regress/sql/partition_prune.sql
index fb1414b9f0..8aa538e496 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -660,6 +660,15 @@ explain (costs off) select * from pp_arrpart where a = 
'{1, 2}';
 explain (costs off) select * from pp_arrpart where a in ('{4, 5}', '{1}');
 drop table pp_arrpart;
 
+-- array type hash partition key
+create table 

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

2018-04-09 Thread Amit Langote
Thanks for the comment.

On 2018/04/09 23:22, Tom Lane wrote:
> Amit Langote  writes:
>> I noticed that the newly added pruning does not work if the partition key
>> is of one of the types that have a corresponding pseudo-type.
> 
> While I don't recall the details due to acute caffeine shortage,
> there are specific coding patterns that are used in the planner
> (e.g. in indxpath.c) to ensure that the code works with pseudotype
> opclasses as well as simple ones.  The existence of this bug
> indicates that those conventions were not followed in the pruning
> code.  I wonder whether this patch makes the pruning code look
> more like the pre-existing code, or even less like it.

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.

Once I changed the code to do it that way, no special considerations are
needed to handle pseudo-type type partition key.

Attached please find the updated patch.

Thanks,
Amit
From 1c136a321153cca04c0842807c2cd166e79f2556 Mon Sep 17 00:00:00 2001
From: amit 
Date: Fri, 8 Dec 2017 19:09:31 +0900
Subject: [PATCH v2] Fix pruning code to determine comparison function
 correctly

It's unreliable to determine one using the constant expression's
type directly (for example, it doesn't work correctly when the
expression contains an array, enum, etc.).  Instead, use righttype
of the operator, the one that supposedly passed the op_in_opfamily
test using the partitioning opfamily.
---
 src/backend/partitioning/partprune.c  | 29 +---
 src/test/regress/expected/partition_prune.out | 98 +++
 src/test/regress/sql/partition_prune.sql  | 44 +++-
 3 files changed, 161 insertions(+), 10 deletions(-)

diff --git a/src/backend/partitioning/partprune.c 
b/src/backend/partitioning/partprune.c
index 7666c6c412..2655d2caa2 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -1426,10 +1426,12 @@ match_clause_to_partition_key(RelOptInfo *rel,
OpExpr *opclause = (OpExpr *) clause;
Expr   *leftop,
   *rightop;
-   Oid commutator = InvalidOid,
+   Oid op_lefttype,
+   op_righttype,
+   commutator = InvalidOid,
negator = InvalidOid;
Oid cmpfn;
-   Oid exprtype;
+   int op_strategy;
boolis_opne_listp = false;
PartClauseInfo *partclause;
 
@@ -1517,10 +1519,20 @@ match_clause_to_partition_key(RelOptInfo *rel,
return PARTCLAUSE_UNSUPPORTED;
}
 
-   /* Check if we're going to need a cross-type comparison 
function. */
-   exprtype = exprType((Node *) expr);
-   if (exprtype != part_scheme->partopcintype[partkeyidx])
+   /*
+* Check if we're going to need a cross-type comparison 
function to
+* use during pruning.
+*/
+   get_op_opfamily_properties(OidIsValid(negator)
+   ? 
negator : opclause->opno,
+  
partopfamily, false,
+  
_strategy, _lefttype, _righttype);
+   /* Use the cached one if no cross-type comparison. */
+   if (op_righttype == part_scheme->partopcintype[partkeyidx])
+   cmpfn = part_scheme->partsupfunc[partkeyidx].fn_oid;
+   else
{
+   /* Otherwise, look the correct one up in the catalog. */
switch (part_scheme->strategy)
{
case PARTITION_STRATEGY_LIST:
@@ -1528,13 +1540,14 @@ match_clause_to_partition_key(RelOptInfo *rel,
cmpfn =

get_opfamily_proc(part_scheme->partopfamily[partkeyidx],

  part_scheme->partopcintype[partkeyidx],
-   
  exprtype, BTORDER_PROC);
+ 

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

2018-04-09 Thread Tom Lane
Amit Langote  writes:
> I noticed that the newly added pruning does not work if the partition key
> is of one of the types that have a corresponding pseudo-type.

While I don't recall the details due to acute caffeine shortage,
there are specific coding patterns that are used in the planner
(e.g. in indxpath.c) to ensure that the code works with pseudotype
opclasses as well as simple ones.  The existence of this bug
indicates that those conventions were not followed in the pruning
code.  I wonder whether this patch makes the pruning code look
more like the pre-existing code, or even less like it.

regards, tom lane



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

2018-04-09 Thread Amit Langote
On 2018/04/09 19:14, Amit Langote wrote:
> Hi.
> 
> I noticed that the newly added pruning does not work if the partition key
> is of one of the types that have a corresponding pseudo-type.
> 
> -- array type list partition key
> create table arrpart (a int[]) partition by list (a);
> create table arrpart1 partition of arrpart for values in ('{1}');
> create table arrpart2 partition of arrpart for values in ('{2, 3}', '{4, 5}');
> explain (costs off) select * from arrpart where a = '{1}';
>QUERY PLAN
> 
>  Append
>->  Seq Scan on arrpart1
>  Filter: (a = '{1}'::integer[])
>->  Seq Scan on arrpart2
>  Filter: (a = '{1}'::integer[])
> (5 rows)
> 
> For pruning, we normally rely on the type's operator class information in
> the system catalogs to be up-to-date, which if it isn't we give up on
> pruning.  For example, if pg_amproc entry for a given type and AM type
> (btree, hash, etc.) has not been populated, we may fail to prune using a
> clause that contains an expression of the said type.  While this is the
> theory for the normal cases, we should make an exception for the
> pseudo-type types.  For those types, we never have pg_amproc entries with
> the "real" type listed.  Instead, the pg_amproc entries contain the
> corresponding pseudo-type.  For example, there aren't pg_amproc entries
> with int4[] (really, its OID) as amproclefttype and/or amprocrighttype,
> instead anyarray is listed there.
> 
> Attached find a patch that tries to fix that and adds relevant tests.

Added to the open items list.

https://wiki.postgresql.org/wiki/PostgreSQL_11_Open_Items#Open_Issues

Thanks,
Amit




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

2018-04-09 Thread Amit Langote
Hi.

I noticed that the newly added pruning does not work if the partition key
is of one of the types that have a corresponding pseudo-type.

-- array type list partition key
create table arrpart (a int[]) partition by list (a);
create table arrpart1 partition of arrpart for values in ('{1}');
create table arrpart2 partition of arrpart for values in ('{2, 3}', '{4, 5}');
explain (costs off) select * from arrpart where a = '{1}';
   QUERY PLAN

 Append
   ->  Seq Scan on arrpart1
 Filter: (a = '{1}'::integer[])
   ->  Seq Scan on arrpart2
 Filter: (a = '{1}'::integer[])
(5 rows)

For pruning, we normally rely on the type's operator class information in
the system catalogs to be up-to-date, which if it isn't we give up on
pruning.  For example, if pg_amproc entry for a given type and AM type
(btree, hash, etc.) has not been populated, we may fail to prune using a
clause that contains an expression of the said type.  While this is the
theory for the normal cases, we should make an exception for the
pseudo-type types.  For those types, we never have pg_amproc entries with
the "real" type listed.  Instead, the pg_amproc entries contain the
corresponding pseudo-type.  For example, there aren't pg_amproc entries
with int4[] (really, its OID) as amproclefttype and/or amprocrighttype,
instead anyarray is listed there.

Attached find a patch that tries to fix that and adds relevant tests.

Thanks,
Amit
From c7945da855973b606b5aa012295e2c0ae93c39c5 Mon Sep 17 00:00:00 2001
From: amit 
Date: Fri, 8 Dec 2017 19:09:31 +0900
Subject: [PATCH v1] Fix pruning when partition key of array, enum, record type

We never have pg_amproc catalog entries with "real" array, enum, record,
range types as leftop and rightop types.  Instead, AM procedures
manipulating such types have entries with the corresponding "pseudo-types"
listed as leftop and rightop types.  For example, for enums, all
procedures entries are marked with anyenum as their leftop and rightop
types.  So, if we pass "real" type OIDs to get_opfamily_member() or
get_opfamily_proc(), we get back an InvalidOid for these type categories.
Whereas we'd normally give up on performing pruning in that case, don't
do that in this case.
---
 src/backend/partitioning/partprune.c  |  4 +-
 src/test/regress/expected/partition_prune.out | 98 +++
 src/test/regress/sql/partition_prune.sql  | 44 +++-
 3 files changed, 144 insertions(+), 2 deletions(-)

diff --git a/src/backend/partitioning/partprune.c 
b/src/backend/partitioning/partprune.c
index 417e1fee81..bd1b99102d 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -1519,7 +1519,9 @@ match_clause_to_partition_key(RelOptInfo *rel,
 
/* Check if we're going to need a cross-type comparison 
function. */
exprtype = exprType((Node *) expr);
-   if (exprtype != part_scheme->partopcintype[partkeyidx])
+   if (exprtype != part_scheme->partopcintype[partkeyidx] &&
+   get_typtype(part_scheme->partopcintype[partkeyidx]) !=
+   TYPTYPE_PSEUDO)
{
switch (part_scheme->strategy)
{
diff --git a/src/test/regress/expected/partition_prune.out 
b/src/test/regress/expected/partition_prune.out
index df3fca025e..69e679d930 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -2399,3 +2399,101 @@ select * from boolp where a = (select value from 
boolvalues where not value);
 
 drop table boolp;
 reset enable_indexonlyscan;
+--
+-- check that pruning works properly when the partition key is of array, enum,
+-- or record type
+--
+-- array type list partition key
+create table arrpart (a int[]) partition by list (a);
+create table arrpart1 partition of arrpart for values in ('{1}');
+create table arrpart2 partition of arrpart for values in ('{2, 3}', '{4, 5}');
+explain (costs off) select * from arrpart where a = '{1}';
+   QUERY PLAN   
+
+ Append
+   ->  Seq Scan on arrpart1
+ Filter: (a = '{1}'::integer[])
+(3 rows)
+
+explain (costs off) select * from arrpart where a = '{1, 2}';
+QUERY PLAN
+--
+ Result
+   One-Time Filter: false
+(2 rows)
+
+explain (costs off) select * from arrpart where a in ('{4, 5}', '{1}');
+  QUERY PLAN  
+--
+ Append
+   ->  Seq Scan on arrpart1
+ Filter: ((a = '{4,5}'::integer[]) OR (a = '{1}'::integer[]))
+   ->  Seq Scan on arrpart2
+ Filter: ((a = '{4,5}'::integer[]) OR (a = '{1}'::integer[]))
+(5 rows)
+
+drop table arrpart;
+-- enum type list partition key