diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c index ad789f6..b86dada 100644 --- a/src/backend/catalog/partition.c +++ b/src/backend/catalog/partition.c @@ -290,7 +290,8 @@ static uint64 compute_hash_value(PartitionKey key, Datum *values, bool *isnull); /* SQL-callable function for use in hash partition CHECK constraints */ PG_FUNCTION_INFO_V1(satisfies_hash_partition); -static Bitmapset *get_partitions_excluded_by_ne_clauses(Relation relation, +static Bitmapset *get_partitions_excluded_by_ne_clauses( + PartitionPruneContext *context, List *ne_clauses); static Bitmapset *get_partitions_from_or_clause_args( PartitionPruneContext *context, @@ -1691,10 +1692,10 @@ get_partition_qual_relid(Oid relid) /* * populate_partition_clause_info * Processes 'clauses' to try to match them to relation's partition - * keys. If any clauses are found which match a partition key, then - * these clauses are stored in 'partclauseinfo'. + * keys. If any compatible clauses are found which match a partition + * key, then these clauses are stored in 'partclauseinfo'. * - * The caller must ensure that 'clauses' is not an empty List. Upon return, + * The caller must ensure that 'clauses' is not an empty List. Upon return, * callers must also check if the 'partclauseinfo' constfalse has been set, if * so, then they must be aware that the 'partclauseinfo' may only be partially * populated. @@ -1747,9 +1748,9 @@ populate_partition_clause_info(Relation relation, } /* - * get_partitions_from_clauses - * Determine all partitions of the context 'relation' that could possibly - * contain a record that matches the context 'clauseinfo' + * get_partitions_from_clauseinfo + * Determine all partitions of the context's 'relation' that could + * possibly contain a record that matches the context's 'clauseinfo' * * Returns a Bitmapset of the matching partition indexes, or NULL if none can * match. @@ -1823,7 +1824,7 @@ get_partitions_from_clauseinfo(PartitionPruneContext *context) { Bitmapset *ne_parts; - ne_parts = get_partitions_excluded_by_ne_clauses(relation, + ne_parts = get_partitions_excluded_by_ne_clauses(context, partclauseinfo->ne_clauses); /* Remove any partitions we found to not be needed */ @@ -1867,11 +1868,13 @@ get_partitions_from_clauseinfo(PartitionPruneContext *context) * possible values that the partition can contain. */ static Bitmapset * -get_partitions_excluded_by_ne_clauses(Relation relation, List *ne_clauses) +get_partitions_excluded_by_ne_clauses(PartitionPruneContext *context, + List *ne_clauses) { ListCell *lc; - Bitmapset *excluded_parts = NULL; + Bitmapset *excluded_parts; Bitmapset *foundoffsets = NULL; + Relation relation = context->relation; PartitionKey partkey = RelationGetPartitionKey(relation); PartitionDesc partdesc = RelationGetPartitionDesc(relation); PartitionBoundInfo boundinfo = partdesc->boundinfo; @@ -1922,10 +1925,10 @@ get_partitions_excluded_by_ne_clauses(Relation relation, List *ne_clauses) * the entire partition. * * We'll need two arrays for this, one to count the number of unique - * datums we found in the query, and another to record the number of - * datums permitted in each partition. Once we've counted all this, we - * can eliminate any partition where the number of datums found matches - * the number of datums allowed in the partition. + * datums found in the query which belong to each partition, and another + * to record the number of datums permitted in each partition. Once we've + * counted all this, we can eliminate any partition where the number of + * datums found matches the number of datums allowed in the partition. */ datums_in_part = (int *) palloc0(sizeof(int) * partdesc->nparts); datums_found = (int *) palloc0(sizeof(int) * partdesc->nparts); @@ -1947,6 +1950,8 @@ get_partitions_excluded_by_ne_clauses(Relation relation, List *ne_clauses) * eliminate the default partition. We can recognize that by it having a * zero value in both arrays. */ + excluded_parts = NULL; + for (i = 0; i < partdesc->nparts; i++) { if (datums_found[i] >= datums_in_part[i] && datums_found[i] > 0) @@ -2183,7 +2188,7 @@ extract_partition_key_clauses(PartitionKey partkey, List *clauses, if (!op_strict(opclause->opno)) continue; - /* Useless if the "constant" can change its value. */ + /* We can't use any volatile value to prune partitions. */ if (contain_volatile_functions((Node *) valueexpr)) continue; @@ -2191,7 +2196,7 @@ extract_partition_key_clauses(PartitionKey partkey, List *clauses, * Handle cases where the clause's operator does not belong to * the partitioning operator family. We currently handle two * such cases: 1. Operators named '<>' are not listed in any - * operator family whatsoever, 2. Ordering opertors like '<' + * operator family whatsoever, 2. Ordering operators like '<' * are not listed in the hash operator families. For 1, check * if list partitioning is in use and if so, proceed to pass * the clause to the caller without doing any more processing @@ -2200,10 +2205,7 @@ extract_partition_key_clauses(PartitionKey partkey, List *clauses, */ if (!op_in_opfamily(opclause->opno, partopfamily)) { - int strategy; - Oid negator, - lefttype, - righttype; + Oid negator; /* * To confirm if the operator is really '<>', check if its @@ -2215,10 +2217,15 @@ extract_partition_key_clauses(PartitionKey partkey, List *clauses, 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 && partkey->strategy == PARTITION_STRATEGY_LIST) is_ne_listp = true; @@ -2244,25 +2251,24 @@ extract_partition_key_clauses(PartitionKey partkey, List *clauses, lappend(partclauseinfo->ne_clauses, pc); else - { partclauseinfo->keyclauses[i] = lappend(partclauseinfo->keyclauses[i], pc); - partclauseinfo->foundkeyclauses = true; - /* - * Since we only allow strict operators, require keys to - * be not null. - */ - if (bms_is_member(i, partclauseinfo->keyisnull)) - { - partclauseinfo->constfalse = true; - return; - } - partclauseinfo->keyisnotnull = + /* + * Since we only allow strict operators, check for any + * contradicting IS NULLs. + */ + if (bms_is_member(i, partclauseinfo->keyisnull)) + { + partclauseinfo->constfalse = true; + return; + } + /* Record that a strict clause has been seen for this key */ + partclauseinfo->keyisnotnull = bms_add_member(partclauseinfo->keyisnotnull, i); - } + partclauseinfo->foundkeyclauses = true; } else if (IsA(clause, ScalarArrayOpExpr)) { diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index 3821977..d4bf973 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -23,7 +23,6 @@ #include "catalog/partition.h" #include "catalog/pg_class.h" #include "catalog/pg_operator.h" -#include "catalog/pg_opfamily.h" #include "catalog/pg_proc.h" #include "foreign/fdwapi.h" #include "miscadmin.h" diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index 2a8cc40..75828a4 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -1510,7 +1510,7 @@ typedef struct OnConflictExpr * PartitionClauseInfo * * Stores clauses which were matched to a partition key. Each matching clause - * is stored in the 'clauses' list for the partition key index that it was + * is stored in the 'keyclauses' list for the partition key index that it was * matched to. Other details are also stored, such as OR clauses and * not-equal (<>) clauses. Nullness properties are also stored. *---------- diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h index 855d51e..4caeaa7 100644 --- a/src/include/nodes/relation.h +++ b/src/include/nodes/relation.h @@ -343,9 +343,8 @@ typedef struct PlannerInfo * input data types are expected to be binary compatible (per ResolveOpClass), * both of those should have same byval and length properties. * - * Since partitioning might be using a collation for a given partition key - * column that is not same as the collation implied by column's type, store - * the same separately. + * The collation of the partition key can differ from the collation of the + * underlying column, so we must store this separately. */ typedef struct PartitionSchemeData { diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out index 83e6081..bc9ff38 100644 --- a/src/test/regress/expected/partition_prune.out +++ b/src/test/regress/expected/partition_prune.out @@ -1366,38 +1366,30 @@ explain (costs off) select * from rp where a <> 1 and a <> 2; Filter: ((a <> 1) AND (a <> 2)) (7 rows) --- various cases for list partitioning where pruning should work -explain (costs off) select * from lp where a <> 'a' and a is not null; - QUERY PLAN ----------------------------------------------------------- +-- null partition should be eliminated due to strict <> clause. +explain (costs off) select * from lp where a <> 'a'; + QUERY PLAN +------------------------------------ Append -> Seq Scan on lp_ad - Filter: ((a IS NOT NULL) AND (a <> 'a'::bpchar)) + Filter: (a <> 'a'::bpchar) -> Seq Scan on lp_bc - Filter: ((a IS NOT NULL) AND (a <> 'a'::bpchar)) + Filter: (a <> 'a'::bpchar) -> Seq Scan on lp_ef - Filter: ((a IS NOT NULL) AND (a <> 'a'::bpchar)) + Filter: (a <> 'a'::bpchar) -> Seq Scan on lp_g - Filter: ((a IS NOT NULL) AND (a <> 'a'::bpchar)) + Filter: (a <> 'a'::bpchar) -> Seq Scan on lp_default - Filter: ((a IS NOT NULL) AND (a <> 'a'::bpchar)) + Filter: (a <> 'a'::bpchar) (11 rows) -explain (costs off) select * from lp where a <> 'a' and a <> 'a'; - QUERY PLAN -------------------------------------------------------------- - Append - -> Seq Scan on lp_ad - Filter: ((a <> 'a'::bpchar) AND (a <> 'a'::bpchar)) - -> Seq Scan on lp_bc - Filter: ((a <> 'a'::bpchar) AND (a <> 'a'::bpchar)) - -> Seq Scan on lp_ef - Filter: ((a <> 'a'::bpchar) AND (a <> 'a'::bpchar)) - -> Seq Scan on lp_g - Filter: ((a <> 'a'::bpchar) AND (a <> 'a'::bpchar)) - -> Seq Scan on lp_default - Filter: ((a <> 'a'::bpchar) AND (a <> 'a'::bpchar)) -(11 rows) +-- ensure we detect contradictions in clauses; a can't be NULL and NOT NULL. +explain (costs off) select * from lp where a <> 'a' and a is null; + QUERY PLAN +-------------------------- + Result + One-Time Filter: false +(2 rows) explain (costs off) select * from lp where (a <> 'a' and a <> 'd') or a is null; QUERY PLAN diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql index 13b1207..b7c5abf 100644 --- a/src/test/regress/sql/partition_prune.sql +++ b/src/test/regress/sql/partition_prune.sql @@ -216,9 +216,12 @@ create table rp2 partition of rp for values from (2) to (maxvalue); explain (costs off) select * from rp where a <> 1; explain (costs off) select * from rp where a <> 1 and a <> 2; --- various cases for list partitioning where pruning should work -explain (costs off) select * from lp where a <> 'a' and a is not null; -explain (costs off) select * from lp where a <> 'a' and a <> 'a'; +-- null partition should be eliminated due to strict <> clause. +explain (costs off) select * from lp where a <> 'a'; + +-- ensure we detect contradictions in clauses; a can't be NULL and NOT NULL. +explain (costs off) select * from lp where a <> 'a' and a is null; + explain (costs off) select * from lp where (a <> 'a' and a <> 'd') or a is null; -- case for list partitioned table that's not root