pruning disabled for array, enum, record, range type partition keys
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
Attachments:
v1-0001-Fix-pruning-when-partition-key-of-array-enum-reco.patchtext/plain; charset=UTF-8; name=v1-0001-Fix-pruning-when-partition-key-of-array-enum-reco.patchDownload
From c7945da855973b606b5aa012295e2c0ae93c39c5 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
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
+create type colors as enum ('green', 'blue', 'black');
+create table enumpart (a colors) partition by list (a);
+create table enumpart_green partition of enumpart for values in ('green');
+create table enumpart_blue partition of enumpart for values in ('blue');
+explain (costs off) select * from enumpart where a = 'blue';
+ QUERY PLAN
+--------------------------------------
+ Append
+ -> Seq Scan on enumpart_blue
+ Filter: (a = 'blue'::colors)
+(3 rows)
+
+explain (costs off) select * from enumpart where a = 'black';
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+drop table enumpart;
+drop type colors;
+-- record type as partition key
+create type rectype as (a int, b int);
+create table recpart (a rectype) partition by list (a);
+create table recpart_11 partition of recpart for values in ('(1,1)');
+create table recpart_23 partition of recpart for values in ('(2,3)');
+explain (costs off) select * from recpart where a = '(1,1)'::rectype;
+ QUERY PLAN
+----------------------------------------
+ Append
+ -> Seq Scan on recpart_11
+ Filter: (a = '(1,1)'::rectype)
+(3 rows)
+
+explain (costs off) select * from recpart where a = '(1,2)'::rectype;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+drop table recpart;
+drop type rectype;
+-- range type partition key
+create table intrangepart (a int4range) partition by list (a);
+create table intrangepart12 partition of intrangepart for values in ('[1,2]');
+create table intrangepart2inf partition of intrangepart for values in ('[2,)');
+explain (costs off) select * from intrangepart where a = '[1,2]'::int4range;
+ QUERY PLAN
+------------------------------------------
+ Append
+ -> Seq Scan on intrangepart12
+ Filter: (a = '[1,3)'::int4range)
+(3 rows)
+
+explain (costs off) select * from intrangepart where a = '(1,2)'::int4range;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+drop table intrangepart;
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
index 7fe93bbc04..587dfb9baf 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -587,4 +587,46 @@ select * from boolp where a = (select value from boolvalues where not value);
drop table boolp;
-reset enable_indexonlyscan;
\ No newline at end of file
+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}';
+explain (costs off) select * from arrpart where a = '{1, 2}';
+explain (costs off) select * from arrpart where a in ('{4, 5}', '{1}');
+drop table arrpart;
+
+-- enum type list partition key
+create type colors as enum ('green', 'blue', 'black');
+create table enumpart (a colors) partition by list (a);
+create table enumpart_green partition of enumpart for values in ('green');
+create table enumpart_blue partition of enumpart for values in ('blue');
+explain (costs off) select * from enumpart where a = 'blue';
+explain (costs off) select * from enumpart where a = 'black';
+drop table enumpart;
+drop type colors;
+
+-- record type as partition key
+create type rectype as (a int, b int);
+create table recpart (a rectype) partition by list (a);
+create table recpart_11 partition of recpart for values in ('(1,1)');
+create table recpart_23 partition of recpart for values in ('(2,3)');
+explain (costs off) select * from recpart where a = '(1,1)'::rectype;
+explain (costs off) select * from recpart where a = '(1,2)'::rectype;
+drop table recpart;
+drop type rectype;
+
+-- range type partition key
+create table intrangepart (a int4range) partition by list (a);
+create table intrangepart12 partition of intrangepart for values in ('[1,2]');
+create table intrangepart2inf partition of intrangepart for values in ('[2,)');
+explain (costs off) select * from intrangepart where a = '[1,2]'::int4range;
+explain (costs off) select * from intrangepart where a = '(1,2)'::int4range;
+drop table intrangepart;
--
2.11.0
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
Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> 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
Thanks for the comment.
On 2018/04/09 23:22, Tom Lane wrote:
Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> 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
Attachments:
v2-0001-Fix-pruning-code-to-determine-comparison-function.patchtext/plain; charset=UTF-8; name=v2-0001-Fix-pruning-code-to-determine-comparison-function.patchDownload
From 1c136a321153cca04c0842807c2cd166e79f2556 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
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;
bool is_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,
+ &op_strategy, &op_lefttype, &op_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);
+ op_righttype, BTORDER_PROC);
break;
case PARTITION_STRATEGY_HASH:
cmpfn =
get_opfamily_proc(part_scheme->partopfamily[partkeyidx],
- exprtype, exprtype, HASHEXTENDED_PROC);
+ op_righttype, op_righttype,
+ HASHEXTENDED_PROC);
break;
default:
@@ -1547,8 +1560,6 @@ match_clause_to_partition_key(RelOptInfo *rel,
if (!OidIsValid(cmpfn))
return PARTCLAUSE_UNSUPPORTED;
}
- else
- cmpfn = part_scheme->partsupfunc[partkeyidx].fn_oid;
partclause = (PartClauseInfo *) palloc(sizeof(PartClauseInfo));
partclause->keyno = partkeyidx;
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index df3fca025e..8874c192ee 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 a array,
+-- enum, record, or range 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
+create type colors as enum ('green', 'blue', 'black');
+create table enumpart (a colors) partition by list (a);
+create table enumpart_green partition of enumpart for values in ('green');
+create table enumpart_blue partition of enumpart for values in ('blue');
+explain (costs off) select * from enumpart where a = 'blue';
+ QUERY PLAN
+--------------------------------------
+ Append
+ -> Seq Scan on enumpart_blue
+ Filter: (a = 'blue'::colors)
+(3 rows)
+
+explain (costs off) select * from enumpart where a = 'black';
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+drop table enumpart;
+drop type colors;
+-- record type as partition key
+create type rectype as (a int, b int);
+create table recpart (a rectype) partition by list (a);
+create table recpart_11 partition of recpart for values in ('(1,1)');
+create table recpart_23 partition of recpart for values in ('(2,3)');
+explain (costs off) select * from recpart where a = '(1,1)'::rectype;
+ QUERY PLAN
+----------------------------------------
+ Append
+ -> Seq Scan on recpart_11
+ Filter: (a = '(1,1)'::rectype)
+(3 rows)
+
+explain (costs off) select * from recpart where a = '(1,2)'::rectype;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+drop table recpart;
+drop type rectype;
+-- range type partition key
+create table intrangepart (a int4range) partition by list (a);
+create table intrangepart12 partition of intrangepart for values in ('[1,2]');
+create table intrangepart2inf partition of intrangepart for values in ('[2,)');
+explain (costs off) select * from intrangepart where a = '[1,2]'::int4range;
+ QUERY PLAN
+------------------------------------------
+ Append
+ -> Seq Scan on intrangepart12
+ Filter: (a = '[1,3)'::int4range)
+(3 rows)
+
+explain (costs off) select * from intrangepart where a = '(1,2)'::int4range;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+drop table intrangepart;
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
index 7fe93bbc04..7e4a69e5a1 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -587,4 +587,46 @@ select * from boolp where a = (select value from boolvalues where not value);
drop table boolp;
-reset enable_indexonlyscan;
\ No newline at end of file
+reset enable_indexonlyscan;
+
+--
+-- check that pruning works properly when the partition key is of a array,
+-- enum, record, or range 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}';
+explain (costs off) select * from arrpart where a = '{1, 2}';
+explain (costs off) select * from arrpart where a in ('{4, 5}', '{1}');
+drop table arrpart;
+
+-- enum type list partition key
+create type colors as enum ('green', 'blue', 'black');
+create table enumpart (a colors) partition by list (a);
+create table enumpart_green partition of enumpart for values in ('green');
+create table enumpart_blue partition of enumpart for values in ('blue');
+explain (costs off) select * from enumpart where a = 'blue';
+explain (costs off) select * from enumpart where a = 'black';
+drop table enumpart;
+drop type colors;
+
+-- record type as partition key
+create type rectype as (a int, b int);
+create table recpart (a rectype) partition by list (a);
+create table recpart_11 partition of recpart for values in ('(1,1)');
+create table recpart_23 partition of recpart for values in ('(2,3)');
+explain (costs off) select * from recpart where a = '(1,1)'::rectype;
+explain (costs off) select * from recpart where a = '(1,2)'::rectype;
+drop table recpart;
+drop type rectype;
+
+-- range type partition key
+create table intrangepart (a int4range) partition by list (a);
+create table intrangepart12 partition of intrangepart for values in ('[1,2]');
+create table intrangepart2inf partition of intrangepart for values in ('[2,)');
+explain (costs off) select * from intrangepart where a = '[1,2]'::int4range;
+explain (costs off) select * from intrangepart where a = '(1,2)'::int4range;
+drop table intrangepart;
--
2.11.0
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 Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
compfuncs-delta.patchtext/plain; charset=us-asciiDownload
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,
&op_strategy, &op_lefttype, &op_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 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}';
+explain (costs off) select * from pph_arrpart where a = '{1, 2}';
+explain (costs off) select * from pph_arrpart where a in ('{4, 5}', '{1}');
+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);
v3-0001-Fix-pruning-code-to-determine-comparison-function.patchtext/plain; charset=us-asciiDownload
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;
bool is_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,
+ &op_strategy, &op_lefttype, &op_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);
+ op_righttype, BTORDER_PROC);
break;
case PARTITION_STRATEGY_HASH:
cmpfn =
get_opfamily_proc(part_scheme->partopfamily[partkeyidx],
- exprtype, exprtype, HASHEXTENDED_PROC);
+ op_righttype, op_righttype,
+ HASHEXTENDED_PROC);
break;
default:
@@ -1547,8 +1560,6 @@ match_clause_to_partition_key(RelOptInfo *rel,
if (!OidIsValid(cmpfn))
return PARTCLAUSE_UNSUPPORTED;
}
- else
- cmpfn = part_scheme->partsupfunc[partkeyidx].fn_oid;
partclause = (PartClauseInfo *) palloc(sizeof(PartClauseInfo));
partclause->keyno = partkeyidx;
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index 844cfb3e42..697a3620a7 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -2599,3 +2599,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 a
+-- pseudotype
+--
+-- array type list partition key
+create table pp_arrpart (a int[]) partition by list (a);
+create table pp_arrpart1 partition of pp_arrpart for values in ('{1}');
+create table pp_arrpart2 partition of pp_arrpart for values in ('{2, 3}', '{4, 5}');
+explain (costs off) select * from pp_arrpart where a = '{1}';
+ QUERY PLAN
+----------------------------------------
+ Append
+ -> Seq Scan on pp_arrpart1
+ Filter: (a = '{1}'::integer[])
+(3 rows)
+
+explain (costs off) select * from pp_arrpart where a = '{1, 2}';
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+explain (costs off) select * from pp_arrpart where a in ('{4, 5}', '{1}');
+ QUERY PLAN
+----------------------------------------------------------------------
+ Append
+ -> Seq Scan on pp_arrpart1
+ Filter: ((a = '{4,5}'::integer[]) OR (a = '{1}'::integer[]))
+ -> Seq Scan on pp_arrpart2
+ Filter: ((a = '{4,5}'::integer[]) OR (a = '{1}'::integer[]))
+(5 rows)
+
+drop table pp_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);
+create table pp_enumpart_green partition of pp_enumpart for values in ('green');
+create table pp_enumpart_blue partition of pp_enumpart for values in ('blue');
+explain (costs off) select * from pp_enumpart where a = 'blue';
+ QUERY PLAN
+-----------------------------------------
+ Append
+ -> Seq Scan on pp_enumpart_blue
+ Filter: (a = 'blue'::pp_colors)
+(3 rows)
+
+explain (costs off) select * from pp_enumpart where a = 'black';
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+drop table pp_enumpart;
+drop type pp_colors;
+-- record type as partition key
+create type pp_rectype as (a int, b int);
+create table pp_recpart (a pp_rectype) partition by list (a);
+create table pp_recpart_11 partition of pp_recpart for values in ('(1,1)');
+create table pp_recpart_23 partition of pp_recpart for values in ('(2,3)');
+explain (costs off) select * from pp_recpart where a = '(1,1)'::pp_rectype;
+ QUERY PLAN
+-------------------------------------------
+ Append
+ -> Seq Scan on pp_recpart_11
+ Filter: (a = '(1,1)'::pp_rectype)
+(3 rows)
+
+explain (costs off) select * from pp_recpart where a = '(1,2)'::pp_rectype;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+drop table pp_recpart;
+drop type pp_rectype;
+-- range type partition key
+create table pp_intrangepart (a int4range) partition by list (a);
+create table pp_intrangepart12 partition of pp_intrangepart for values in ('[1,2]');
+create table pp_intrangepart2inf partition of pp_intrangepart for values in ('[2,)');
+explain (costs off) select * from pp_intrangepart where a = '[1,2]'::int4range;
+ QUERY PLAN
+------------------------------------------
+ Append
+ -> Seq Scan on pp_intrangepart12
+ Filter: (a = '[1,3)'::int4range)
+(3 rows)
+
+explain (costs off) select * from pp_intrangepart where a = '(1,2)'::int4range;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+drop table pp_intrangepart;
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
index e808d1a439..fb1414b9f0 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -645,3 +645,45 @@ 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 a
+-- pseudotype
+--
+
+-- array type list partition key
+create table pp_arrpart (a int[]) partition by list (a);
+create table pp_arrpart1 partition of pp_arrpart for values in ('{1}');
+create table pp_arrpart2 partition of pp_arrpart for values in ('{2, 3}', '{4, 5}');
+explain (costs off) select * from pp_arrpart where a = '{1}';
+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;
+
+-- 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);
+create table pp_enumpart_green partition of pp_enumpart for values in ('green');
+create table pp_enumpart_blue partition of pp_enumpart for values in ('blue');
+explain (costs off) select * from pp_enumpart where a = 'blue';
+explain (costs off) select * from pp_enumpart where a = 'black';
+drop table pp_enumpart;
+drop type pp_colors;
+
+-- record type as partition key
+create type pp_rectype as (a int, b int);
+create table pp_recpart (a pp_rectype) partition by list (a);
+create table pp_recpart_11 partition of pp_recpart for values in ('(1,1)');
+create table pp_recpart_23 partition of pp_recpart for values in ('(2,3)');
+explain (costs off) select * from pp_recpart where a = '(1,1)'::pp_rectype;
+explain (costs off) select * from pp_recpart where a = '(1,2)'::pp_rectype;
+drop table pp_recpart;
+drop type pp_rectype;
+
+-- range type partition key
+create table pp_intrangepart (a int4range) partition by list (a);
+create table pp_intrangepart12 partition of pp_intrangepart for values in ('[1,2]');
+create table pp_intrangepart2inf partition of pp_intrangepart for values in ('[2,)');
+explain (costs off) select * from pp_intrangepart where a = '[1,2]'::int4range;
+explain (costs off) select * from pp_intrangepart where a = '(1,2)'::int4range;
+drop table pp_intrangepart;
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
Attachments:
v4-0001-Fix-pruning-code-to-determine-comparison-function.patchtext/plain; charset=UTF-8; name=v4-0001-Fix-pruning-code-to-determine-comparison-function.patchDownload
From d6652eb932a5e1cf6b63fa1dd09e4dd752267dbc Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
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;
bool is_opne_listp = false;
PartClauseInfo *partclause;
@@ -1517,24 +1519,51 @@ 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 can perform pruning if the clause's other argument is
+ * not of the same type as the partitioning opclass's declared input
+ * type. If it *is* of the same type, we can simply use the cached
+ * procedure, that is, the one in PartitionKey. If not, we must find
+ * the correct procedure in pg_amproc, which if it doesn't exist, we
+ * have to give up on pruning using this clause.
+ */
+ get_op_opfamily_properties(OidIsValid(negator) ?
+ negator : opclause->opno,
+ partopfamily, false,
+ &op_strategy, &op_lefttype, &op_righttype);
+
+ if (op_righttype == part_scheme->partopcintype[partkeyidx])
+ cmpfn = part_scheme->partsupfunc[partkeyidx].fn_oid;
+ else
{
switch (part_scheme->strategy)
{
+ /*
+ * RANGE and LIST partitioning use a btree operator family,
+ * wherein, we can expect to find a procedure in pg_amproc
+ * whose righttype matches the clause operator's righttype.
+ */
case PARTITION_STRATEGY_LIST:
case PARTITION_STRATEGY_RANGE:
cmpfn =
get_opfamily_proc(part_scheme->partopfamily[partkeyidx],
part_scheme->partopcintype[partkeyidx],
- exprtype, BTORDER_PROC);
+ op_righttype, BTORDER_PROC);
break;
+ /*
+ * HASH partitioning uses a hash operator family, wherein the
+ * support function only ever processes *one* input value that
+ * is to be hashed. So, it doesn't make sense to look up
+ * using two distinct type OIDs. Instead, pass the desired
+ * type's OID as both lefttype and righttype when searching in
+ * pg_amproc.
+ */
case PARTITION_STRATEGY_HASH:
cmpfn =
get_opfamily_proc(part_scheme->partopfamily[partkeyidx],
- exprtype, exprtype, HASHEXTENDED_PROC);
+ op_righttype, op_righttype,
+ HASHEXTENDED_PROC);
break;
default:
@@ -1547,8 +1576,6 @@ match_clause_to_partition_key(RelOptInfo *rel,
if (!OidIsValid(cmpfn))
return PARTCLAUSE_UNSUPPORTED;
}
- else
- cmpfn = part_scheme->partsupfunc[partkeyidx].fn_oid;
partclause = (PartClauseInfo *) palloc(sizeof(PartClauseInfo));
partclause->keyno = partkeyidx;
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index 844cfb3e42..9c65ee001d 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -2599,3 +2599,141 @@ 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 a
+-- pseudotype
+--
+-- array type list partition key
+create table pp_arrpart (a int[]) partition by list (a);
+create table pp_arrpart1 partition of pp_arrpart for values in ('{1}');
+create table pp_arrpart2 partition of pp_arrpart for values in ('{2, 3}', '{4, 5}');
+explain (costs off) select * from pp_arrpart where a = '{1}';
+ QUERY PLAN
+----------------------------------------
+ Append
+ -> Seq Scan on pp_arrpart1
+ Filter: (a = '{1}'::integer[])
+(3 rows)
+
+explain (costs off) select * from pp_arrpart where a = '{1, 2}';
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+explain (costs off) select * from pp_arrpart where a in ('{4, 5}', '{1}');
+ QUERY PLAN
+----------------------------------------------------------------------
+ Append
+ -> Seq Scan on pp_arrpart1
+ Filter: ((a = '{4,5}'::integer[]) OR (a = '{1}'::integer[]))
+ -> Seq Scan on pp_arrpart2
+ Filter: ((a = '{4,5}'::integer[]) OR (a = '{1}'::integer[]))
+(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);
+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)
+
+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);
+create table pp_enumpart_green partition of pp_enumpart for values in ('green');
+create table pp_enumpart_blue partition of pp_enumpart for values in ('blue');
+explain (costs off) select * from pp_enumpart where a = 'blue';
+ QUERY PLAN
+-----------------------------------------
+ Append
+ -> Seq Scan on pp_enumpart_blue
+ Filter: (a = 'blue'::pp_colors)
+(3 rows)
+
+explain (costs off) select * from pp_enumpart where a = 'black';
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+drop table pp_enumpart;
+drop type pp_colors;
+-- record type as partition key
+create type pp_rectype as (a int, b int);
+create table pp_recpart (a pp_rectype) partition by list (a);
+create table pp_recpart_11 partition of pp_recpart for values in ('(1,1)');
+create table pp_recpart_23 partition of pp_recpart for values in ('(2,3)');
+explain (costs off) select * from pp_recpart where a = '(1,1)'::pp_rectype;
+ QUERY PLAN
+-------------------------------------------
+ Append
+ -> Seq Scan on pp_recpart_11
+ Filter: (a = '(1,1)'::pp_rectype)
+(3 rows)
+
+explain (costs off) select * from pp_recpart where a = '(1,2)'::pp_rectype;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+drop table pp_recpart;
+drop type pp_rectype;
+-- range type partition key
+create table pp_intrangepart (a int4range) partition by list (a);
+create table pp_intrangepart12 partition of pp_intrangepart for values in ('[1,2]');
+create table pp_intrangepart2inf partition of pp_intrangepart for values in ('[2,)');
+explain (costs off) select * from pp_intrangepart where a = '[1,2]'::int4range;
+ QUERY PLAN
+------------------------------------------
+ Append
+ -> Seq Scan on pp_intrangepart12
+ Filter: (a = '[1,3)'::int4range)
+(3 rows)
+
+explain (costs off) select * from pp_intrangepart where a = '(1,2)'::int4range;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+drop table pp_intrangepart;
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
index e808d1a439..b38b39c71e 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -645,3 +645,56 @@ 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 a
+-- pseudotype
+--
+
+-- array type list partition key
+create table pp_arrpart (a int[]) partition by list (a);
+create table pp_arrpart1 partition of pp_arrpart for values in ('{1}');
+create table pp_arrpart2 partition of pp_arrpart for values in ('{2, 3}', '{4, 5}');
+explain (costs off) select * from pp_arrpart where a = '{1}';
+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 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;
+explain (costs off) select * from pph_arrpart where a = '{1}';
+explain (costs off) select * from pph_arrpart where a = '{1, 2}';
+explain (costs off) select * from pph_arrpart where a in ('{4, 5}', '{1}');
+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);
+create table pp_enumpart_green partition of pp_enumpart for values in ('green');
+create table pp_enumpart_blue partition of pp_enumpart for values in ('blue');
+explain (costs off) select * from pp_enumpart where a = 'blue';
+explain (costs off) select * from pp_enumpart where a = 'black';
+drop table pp_enumpart;
+drop type pp_colors;
+
+-- record type as partition key
+create type pp_rectype as (a int, b int);
+create table pp_recpart (a pp_rectype) partition by list (a);
+create table pp_recpart_11 partition of pp_recpart for values in ('(1,1)');
+create table pp_recpart_23 partition of pp_recpart for values in ('(2,3)');
+explain (costs off) select * from pp_recpart where a = '(1,1)'::pp_rectype;
+explain (costs off) select * from pp_recpart where a = '(1,2)'::pp_rectype;
+drop table pp_recpart;
+drop type pp_rectype;
+
+-- range type partition key
+create table pp_intrangepart (a int4range) partition by list (a);
+create table pp_intrangepart12 partition of pp_intrangepart for values in ('[1,2]');
+create table pp_intrangepart2inf partition of pp_intrangepart for values in ('[2,)');
+explain (costs off) select * from pp_intrangepart where a = '[1,2]'::int4range;
+explain (costs off) select * from pp_intrangepart where a = '(1,2)'::int4range;
+drop table pp_intrangepart;
--
2.11.0
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 Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Thu, Apr 19, 2018 at 12:01 AM, Alvaro Herrera
<alvherre@alvh.no-ip.org> 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
Amit Langote wrote:
On Thu, Apr 19, 2018 at 12:01 AM, Alvaro Herrera
<alvherre@alvh.no-ip.org> 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 Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
v5-0001-Fix-pruning-code-to-determine-comparison-function.patchtext/plain; charset=us-asciiDownload
From 340b0b3a97af0fa07562bc4434a4908402c3efbd Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
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;
bool is_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,
+ &op_strategy, &op_lefttype,
+ &op_righttype);
+ }
+ else
+ {
+ /* Not good unless list partitioning */
if (part_scheme->strategy != PARTITION_STRATEGY_LIST)
return PARTCLAUSE_UNSUPPORTED;
- /*
- * To confirm if the operator is really '<>', check if its negator
- * is a btree equality operator.
- */
+ /* See if the negator is equality */
negator = get_negator(opclause->opno);
if (OidIsValid(negator) && op_in_opfamily(negator, partopfamily))
{
- Oid lefttype;
- Oid righttype;
- int strategy;
-
get_op_opfamily_properties(negator, partopfamily, false,
- &strategy, &lefttype, &righttype);
-
- if (strategy == BTEqualStrategyNumber)
- is_opne_listp = true;
+ &op_strategy, &op_lefttype,
+ &op_righttype);
+ if (op_strategy == BTEqualStrategyNumber)
+ is_opne_listp = true; /* bingo */
}
/* Operator isn't really what we were hoping it'd be. */
@@ -1517,24 +1525,41 @@ 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])
+ /*
+ * Now find the procedure to use, based on the types. If the clause's
+ * other argument is of the same type as the partitioning opclass's
+ * declared input type, we can use the procedure cached in
+ * PartitionKey. If not, search for a cross-type one in the same
+ * opfamily; if one doesn't exist, give up on pruning for this clause.
+ */
+ if (op_righttype == part_scheme->partopcintype[partkeyidx])
+ cmpfn = part_scheme->partsupfunc[partkeyidx].fn_oid;
+ else
{
switch (part_scheme->strategy)
{
+ /*
+ * For range and list partitioning, we need the ordering
+ * procedure with lefttype being the partition key's type, and
+ * righttype the clause's operator's right type.
+ */
case PARTITION_STRATEGY_LIST:
case PARTITION_STRATEGY_RANGE:
cmpfn =
get_opfamily_proc(part_scheme->partopfamily[partkeyidx],
part_scheme->partopcintype[partkeyidx],
- exprtype, BTORDER_PROC);
+ op_righttype, BTORDER_PROC);
break;
+ /*
+ * For hash partitioning, we need the hashing procedure for
+ * the clause's type.
+ */
case PARTITION_STRATEGY_HASH:
cmpfn =
get_opfamily_proc(part_scheme->partopfamily[partkeyidx],
- exprtype, exprtype, HASHEXTENDED_PROC);
+ op_righttype, op_righttype,
+ HASHEXTENDED_PROC);
break;
default:
@@ -1547,34 +1572,26 @@ match_clause_to_partition_key(RelOptInfo *rel,
if (!OidIsValid(cmpfn))
return PARTCLAUSE_UNSUPPORTED;
}
- else
- cmpfn = part_scheme->partsupfunc[partkeyidx].fn_oid;
+ /*
+ * Build the clause, passing the negator or commutator if applicable.
+ */
partclause = (PartClauseInfo *) palloc(sizeof(PartClauseInfo));
partclause->keyno = partkeyidx;
-
- /* For <> operator clauses, pass on the negator. */
- partclause->op_is_ne = false;
- partclause->op_strategy = InvalidStrategy;
-
if (is_opne_listp)
{
Assert(OidIsValid(negator));
partclause->opno = negator;
partclause->op_is_ne = true;
-
- /*
- * We already know the strategy in this case, so may as well set
- * it rather than having to look it up later.
- */
partclause->op_strategy = BTEqualStrategyNumber;
}
- /* And if commuted before matching, pass on the commutator */
- else if (OidIsValid(commutator))
- partclause->opno = commutator;
else
- partclause->opno = opclause->opno;
-
+ {
+ partclause->opno = OidIsValid(commutator) ?
+ commutator : opclause->opno;
+ partclause->op_is_ne = false;
+ partclause->op_strategy = op_strategy;
+ }
partclause->expr = expr;
partclause->cmpfn = cmpfn;
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index 844cfb3e42..9c65ee001d 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -2599,3 +2599,141 @@ 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 a
+-- pseudotype
+--
+-- array type list partition key
+create table pp_arrpart (a int[]) partition by list (a);
+create table pp_arrpart1 partition of pp_arrpart for values in ('{1}');
+create table pp_arrpart2 partition of pp_arrpart for values in ('{2, 3}', '{4, 5}');
+explain (costs off) select * from pp_arrpart where a = '{1}';
+ QUERY PLAN
+----------------------------------------
+ Append
+ -> Seq Scan on pp_arrpart1
+ Filter: (a = '{1}'::integer[])
+(3 rows)
+
+explain (costs off) select * from pp_arrpart where a = '{1, 2}';
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+explain (costs off) select * from pp_arrpart where a in ('{4, 5}', '{1}');
+ QUERY PLAN
+----------------------------------------------------------------------
+ Append
+ -> Seq Scan on pp_arrpart1
+ Filter: ((a = '{4,5}'::integer[]) OR (a = '{1}'::integer[]))
+ -> Seq Scan on pp_arrpart2
+ Filter: ((a = '{4,5}'::integer[]) OR (a = '{1}'::integer[]))
+(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);
+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)
+
+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);
+create table pp_enumpart_green partition of pp_enumpart for values in ('green');
+create table pp_enumpart_blue partition of pp_enumpart for values in ('blue');
+explain (costs off) select * from pp_enumpart where a = 'blue';
+ QUERY PLAN
+-----------------------------------------
+ Append
+ -> Seq Scan on pp_enumpart_blue
+ Filter: (a = 'blue'::pp_colors)
+(3 rows)
+
+explain (costs off) select * from pp_enumpart where a = 'black';
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+drop table pp_enumpart;
+drop type pp_colors;
+-- record type as partition key
+create type pp_rectype as (a int, b int);
+create table pp_recpart (a pp_rectype) partition by list (a);
+create table pp_recpart_11 partition of pp_recpart for values in ('(1,1)');
+create table pp_recpart_23 partition of pp_recpart for values in ('(2,3)');
+explain (costs off) select * from pp_recpart where a = '(1,1)'::pp_rectype;
+ QUERY PLAN
+-------------------------------------------
+ Append
+ -> Seq Scan on pp_recpart_11
+ Filter: (a = '(1,1)'::pp_rectype)
+(3 rows)
+
+explain (costs off) select * from pp_recpart where a = '(1,2)'::pp_rectype;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+drop table pp_recpart;
+drop type pp_rectype;
+-- range type partition key
+create table pp_intrangepart (a int4range) partition by list (a);
+create table pp_intrangepart12 partition of pp_intrangepart for values in ('[1,2]');
+create table pp_intrangepart2inf partition of pp_intrangepart for values in ('[2,)');
+explain (costs off) select * from pp_intrangepart where a = '[1,2]'::int4range;
+ QUERY PLAN
+------------------------------------------
+ Append
+ -> Seq Scan on pp_intrangepart12
+ Filter: (a = '[1,3)'::int4range)
+(3 rows)
+
+explain (costs off) select * from pp_intrangepart where a = '(1,2)'::int4range;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+drop table pp_intrangepart;
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
index e808d1a439..b38b39c71e 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -645,3 +645,56 @@ 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 a
+-- pseudotype
+--
+
+-- array type list partition key
+create table pp_arrpart (a int[]) partition by list (a);
+create table pp_arrpart1 partition of pp_arrpart for values in ('{1}');
+create table pp_arrpart2 partition of pp_arrpart for values in ('{2, 3}', '{4, 5}');
+explain (costs off) select * from pp_arrpart where a = '{1}';
+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 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;
+explain (costs off) select * from pph_arrpart where a = '{1}';
+explain (costs off) select * from pph_arrpart where a = '{1, 2}';
+explain (costs off) select * from pph_arrpart where a in ('{4, 5}', '{1}');
+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);
+create table pp_enumpart_green partition of pp_enumpart for values in ('green');
+create table pp_enumpart_blue partition of pp_enumpart for values in ('blue');
+explain (costs off) select * from pp_enumpart where a = 'blue';
+explain (costs off) select * from pp_enumpart where a = 'black';
+drop table pp_enumpart;
+drop type pp_colors;
+
+-- record type as partition key
+create type pp_rectype as (a int, b int);
+create table pp_recpart (a pp_rectype) partition by list (a);
+create table pp_recpart_11 partition of pp_recpart for values in ('(1,1)');
+create table pp_recpart_23 partition of pp_recpart for values in ('(2,3)');
+explain (costs off) select * from pp_recpart where a = '(1,1)'::pp_rectype;
+explain (costs off) select * from pp_recpart where a = '(1,2)'::pp_rectype;
+drop table pp_recpart;
+drop type pp_rectype;
+
+-- range type partition key
+create table pp_intrangepart (a int4range) partition by list (a);
+create table pp_intrangepart12 partition of pp_intrangepart for values in ('[1,2]');
+create table pp_intrangepart2inf partition of pp_intrangepart for values in ('[2,)');
+explain (costs off) select * from pp_intrangepart where a = '[1,2]'::int4range;
+explain (costs off) select * from pp_intrangepart where a = '(1,2)'::int4range;
+drop table pp_intrangepart;
--
2.11.0
Alvaro Herrera <alvherre@alvh.no-ip.org> 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
Hi.
On 2018/04/19 6:45, Alvaro Herrera wrote:
Amit Langote wrote:
On Thu, Apr 19, 2018 at 12:01 AM, Alvaro Herrera
<alvherre@alvh.no-ip.org> 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
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 Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services