diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c index d3868b628d..032aa02406 100644 --- a/src/backend/optimizer/plan/initsplan.c +++ b/src/backend/optimizer/plan/initsplan.c @@ -31,6 +31,7 @@ #include "parser/analyze.h" #include "rewrite/rewriteManip.h" #include "utils/lsyscache.h" +#include "utils/rel.h" #include "utils/typcache.h" /* These parameters are set by GUC */ @@ -2629,32 +2630,51 @@ add_base_clause_to_rel(PlannerInfo *root, Index relid, RestrictInfo *restrictinfo) { RelOptInfo *rel = find_base_rel(root, relid); + RangeTblEntry *rte = root->simple_rte_array[relid]; Assert(bms_membership(restrictinfo->required_relids) == BMS_SINGLETON); - /* Don't add the clause if it is always true */ - if (restriction_is_always_true(root, restrictinfo)) - return; - /* - * Substitute the origin qual with constant-FALSE if it is provably always - * false. Note that we keep the same rinfo_serial. + * For inheritance parent tables, we must always record the RestrictInfo + * in baserestrictinfo as is. If we were to transform or skip adding it, + * then the original wouldn't be available in apply_child_basequals. Since + * there are two RangeTblEntries for inheritance parents, one with + * inh==true and the other with inh==false, we're still able to apply this + * optimization to the inh==false one. The inh==true one is what + * apply_child_basequals() sees, whereas the inh==false one is what's used + * for the scan node in the final plan. + * + * We make an exception to this is for partitioned tables. For these, we + * always apply the constant-TRUE and constant-FALSE transformations. A + * qual which is either of these for a partitioned table must also be that + * for all of its child partitions. */ - if (restriction_is_always_false(root, restrictinfo)) + if (!rte->inh || rte->relkind == RELKIND_PARTITIONED_TABLE) { - int save_rinfo_serial = restrictinfo->rinfo_serial; + /* Don't add the clause if it is always true */ + if (restriction_is_always_true(root, restrictinfo)) + return; - restrictinfo = make_restrictinfo(root, - (Expr *) makeBoolConst(false, false), - restrictinfo->is_pushed_down, - restrictinfo->has_clone, - restrictinfo->is_clone, - restrictinfo->pseudoconstant, - 0, /* security_level */ - restrictinfo->required_relids, - restrictinfo->incompatible_relids, - restrictinfo->outer_relids); - restrictinfo->rinfo_serial = save_rinfo_serial; + /* + * Substitute the origin qual with constant-FALSE if it is provably + * always false. Note that we keep the same rinfo_serial. + */ + if (restriction_is_always_false(root, restrictinfo)) + { + int save_rinfo_serial = restrictinfo->rinfo_serial; + + restrictinfo = make_restrictinfo(root, + (Expr *) makeBoolConst(false, false), + restrictinfo->is_pushed_down, + restrictinfo->has_clone, + restrictinfo->is_clone, + restrictinfo->pseudoconstant, + 0, /* security_level */ + restrictinfo->required_relids, + restrictinfo->incompatible_relids, + restrictinfo->outer_relids); + restrictinfo->rinfo_serial = save_rinfo_serial; + } } /* Add clause to rel's restriction list */ diff --git a/src/backend/optimizer/util/inherit.c b/src/backend/optimizer/util/inherit.c index 5c7acf8a90..4797312ae5 100644 --- a/src/backend/optimizer/util/inherit.c +++ b/src/backend/optimizer/util/inherit.c @@ -822,11 +822,13 @@ expand_appendrel_subquery(PlannerInfo *root, RelOptInfo *rel, /* * apply_child_basequals * Populate childrel's base restriction quals from parent rel's quals, - * translating them using appinfo. + * translating Vars using appinfo and re-checking for quals which are + * constant-TRUE or constant-FALSE when applied to this child relation. * * If any of the resulting clauses evaluate to constant false or NULL, we * return false and don't apply any quals. Caller should mark the relation as - * a dummy rel in this case, since it doesn't need to be scanned. + * a dummy rel in this case, since it doesn't need to be scanned. Constant + * true quals are ignored. */ bool apply_child_basequals(PlannerInfo *root, RelOptInfo *parentrel, @@ -875,6 +877,7 @@ apply_child_basequals(PlannerInfo *root, RelOptInfo *parentrel, { Node *onecq = (Node *) lfirst(lc2); bool pseudoconstant; + RestrictInfo *childrinfo; /* check for pseudoconstant (no Vars or volatile functions) */ pseudoconstant = @@ -886,15 +889,23 @@ apply_child_basequals(PlannerInfo *root, RelOptInfo *parentrel, root->hasPseudoConstantQuals = true; } /* reconstitute RestrictInfo with appropriate properties */ - childquals = lappend(childquals, - make_restrictinfo(root, - (Expr *) onecq, - rinfo->is_pushed_down, - rinfo->has_clone, - rinfo->is_clone, - pseudoconstant, - rinfo->security_level, - NULL, NULL, NULL)); + childrinfo = make_restrictinfo(root, + (Expr *) onecq, + rinfo->is_pushed_down, + rinfo->has_clone, + rinfo->is_clone, + pseudoconstant, + rinfo->security_level, + NULL, NULL, NULL); + + /* Restriction is proven always false */ + if (restriction_is_always_false(root, childrinfo)) + return false; + /* Restriction is proven always true, so drop it */ + if (restriction_is_always_true(root, childrinfo)) + continue; + + childquals = lappend(childquals, childrinfo); /* track minimum security level among child quals */ cq_min_security = Min(cq_min_security, rinfo->security_level); } diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c index 6bb53e4346..130f838629 100644 --- a/src/backend/optimizer/util/plancat.c +++ b/src/backend/optimizer/util/plancat.c @@ -161,22 +161,33 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent, rel->attr_widths = (int32 *) palloc0((rel->max_attr - rel->min_attr + 1) * sizeof(int32)); - /* record which columns are defined as NOT NULL */ - for (int i = 0; i < relation->rd_att->natts; i++) + /* + * Record which columns are defined as NOT NULL. We leave this + * unpopulated for non-partitioned inheritance parent relations as it's + * ambiguous as to what it means. Some child tables may have a NOT NULL + * constraint for a column while others may not. We could work harder and + * build a unioned set of all child relations notnullattnums, but there's + * currently no need. The RelOptInfo corresponding to the !inh + * RangeTblEntry does get populated. + */ + if (!inhparent || relation->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { - FormData_pg_attribute *attr = &relation->rd_att->attrs[i]; - - if (attr->attnotnull) + for (int i = 0; i < relation->rd_att->natts; i++) { - rel->notnullattnums = bms_add_member(rel->notnullattnums, - attr->attnum); + FormData_pg_attribute *attr = &relation->rd_att->attrs[i]; - /* - * Per RemoveAttributeById(), dropped columns will have their - * attnotnull unset, so we needn't check for dropped columns in - * the above condition. - */ - Assert(!attr->attisdropped); + if (attr->attnotnull) + { + rel->notnullattnums = bms_add_member(rel->notnullattnums, + attr->attnum); + + /* + * Per RemoveAttributeById(), dropped columns will have their + * attnotnull unset, so we needn't check for dropped columns + * in the above condition. + */ + Assert(!attr->attisdropped); + } } } diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c index d791c4108d..0c125e42e8 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -373,10 +373,20 @@ build_simple_rel(PlannerInfo *root, int relid, RelOptInfo *parent) } /* - * Copy the parent's quals to the child, with appropriate substitution of - * variables. If any constant false or NULL clauses turn up, we can mark - * the child as dummy right away. (We must do this immediately so that - * pruning works correctly when recursing in expand_partitioned_rtentry.) + * We must apply the partially filled in RelOptInfo before calling + * apply_child_basequals due to some transformations within that function + * which require the RelOptInfo to be available in the simple_rel_array. + */ + root->simple_rel_array[relid] = rel; + + /* + * Apply the parent's quals to the child, with appropriate substitution of + * variables. If the resulting clause is constant-FALSE or NULL after + * applying transformations, apply_child_basequals returns false to + * indicate that scanning this relation won't yield any rows. In this + * case, we mark the child as dummy right away. (We must do this + * immediately so that pruning works correctly when recursing in + * expand_partitioned_rtentry.) */ if (parent) { @@ -386,16 +396,13 @@ build_simple_rel(PlannerInfo *root, int relid, RelOptInfo *parent) if (!apply_child_basequals(root, parent, rel, rte, appinfo)) { /* - * Some restriction clause reduced to constant FALSE or NULL after - * substitution, so this child need not be scanned. + * Restriction clause reduced to constant FALSE or NULL. Mark as + * dummy so we won't scan this relation. */ mark_dummy_rel(rel); } } - /* Save the finished struct in the query's simple_rel_array */ - root->simple_rel_array[relid] = rel; - return rel; } diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index 0ab25d9ce7..6c71098f2d 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -918,7 +918,11 @@ typedef struct RelOptInfo Relids *attr_needed pg_node_attr(read_write_ignore); /* array indexed [min_attr .. max_attr] */ int32 *attr_widths pg_node_attr(read_write_ignore); - /* zero-based set containing attnums of NOT NULL columns */ + + /* + * Zero-based set containing attnums of NOT NULL columns. Not populated + * for rels corresponding to non-partitioned inh==true RTEs. + */ Bitmapset *notnullattnums; /* relids of outer joins that can null this baserel */ Relids nulling_relids; diff --git a/src/test/regress/expected/predicate.out b/src/test/regress/expected/predicate.out index 60bf3e37aa..6f1cc0d54c 100644 --- a/src/test/regress/expected/predicate.out +++ b/src/test/regress/expected/predicate.out @@ -242,3 +242,51 @@ SELECT * FROM pred_tab t1 (9 rows) DROP TABLE pred_tab; +-- Validate we handle IS NULL and IS NOT NULL quals correctly with inheritance +-- parents. +CREATE TABLE pred_parent (a int); +CREATE TABLE pred_child () INHERITS (pred_parent); +ALTER TABLE ONLY pred_parent ALTER a SET NOT NULL; +-- Ensure that the scan on pred_child contains the IS NOT NULL qual. +EXPLAIN (COSTS OFF) +SELECT * FROM pred_parent WHERE a IS NOT NULL; + QUERY PLAN +--------------------------------------------- + Append + -> Seq Scan on pred_parent pred_parent_1 + -> Seq Scan on pred_child pred_parent_2 + Filter: (a IS NOT NULL) +(4 rows) + +-- Ensure we only scan pred_child and not pred_parent +EXPLAIN (COSTS OFF) +SELECT * FROM pred_parent WHERE a IS NULL; + QUERY PLAN +------------------------------------ + Seq Scan on pred_child pred_parent + Filter: (a IS NULL) +(2 rows) + +ALTER TABLE pred_parent ALTER a DROP NOT NULL; +ALTER TABLE pred_child ALTER a SET NOT NULL; +-- Ensure the IS NOT NULL qual is removed from the pred_child scan. +EXPLAIN (COSTS OFF) +SELECT * FROM pred_parent WHERE a IS NOT NULL; + QUERY PLAN +--------------------------------------------- + Append + -> Seq Scan on pred_parent pred_parent_1 + Filter: (a IS NOT NULL) + -> Seq Scan on pred_child pred_parent_2 +(4 rows) + +-- Ensure we only scan pred_parent and not pred_child +EXPLAIN (COSTS OFF) +SELECT * FROM pred_parent WHERE a IS NULL; + QUERY PLAN +------------------------- + Seq Scan on pred_parent + Filter: (a IS NULL) +(2 rows) + +DROP TABLE pred_parent, pred_child; diff --git a/src/test/regress/sql/predicate.sql b/src/test/regress/sql/predicate.sql index 563e395fde..63f6a7786f 100644 --- a/src/test/regress/sql/predicate.sql +++ b/src/test/regress/sql/predicate.sql @@ -120,3 +120,30 @@ SELECT * FROM pred_tab t1 LEFT JOIN pred_tab t3 ON t2.a IS NULL OR t2.c IS NULL; DROP TABLE pred_tab; + +-- Validate we handle IS NULL and IS NOT NULL quals correctly with inheritance +-- parents. +CREATE TABLE pred_parent (a int); +CREATE TABLE pred_child () INHERITS (pred_parent); +ALTER TABLE ONLY pred_parent ALTER a SET NOT NULL; + +-- Ensure that the scan on pred_child contains the IS NOT NULL qual. +EXPLAIN (COSTS OFF) +SELECT * FROM pred_parent WHERE a IS NOT NULL; + +-- Ensure we only scan pred_child and not pred_parent +EXPLAIN (COSTS OFF) +SELECT * FROM pred_parent WHERE a IS NULL; + +ALTER TABLE pred_parent ALTER a DROP NOT NULL; +ALTER TABLE pred_child ALTER a SET NOT NULL; + +-- Ensure the IS NOT NULL qual is removed from the pred_child scan. +EXPLAIN (COSTS OFF) +SELECT * FROM pred_parent WHERE a IS NOT NULL; + +-- Ensure we only scan pred_parent and not pred_child +EXPLAIN (COSTS OFF) +SELECT * FROM pred_parent WHERE a IS NULL; + +DROP TABLE pred_parent, pred_child;