From ffd6e33c7e897ab975a347aeb2f9b39c185f5d81 Mon Sep 17 00:00:00 2001 From: Richard Guo Date: Tue, 9 Apr 2024 16:49:59 +0800 Subject: [PATCH v1] Fix handling of IS [NOT] NULL quals on inheritance parents --- src/backend/optimizer/util/inherit.c | 37 ++++++++++----- src/backend/optimizer/util/plancat.c | 42 +++++++++++------ src/backend/optimizer/util/relnode.c | 26 +++++++---- src/test/regress/expected/predicate.out | 60 +++++++++++++++++++++++++ src/test/regress/sql/predicate.sql | 42 +++++++++++++++++ 5 files changed, 174 insertions(+), 33 deletions(-) diff --git a/src/backend/optimizer/util/inherit.c b/src/backend/optimizer/util/inherit.c index 5c7acf8a90..4f594f4f65 100644 --- a/src/backend/optimizer/util/inherit.c +++ b/src/backend/optimizer/util/inherit.c @@ -824,9 +824,12 @@ expand_appendrel_subquery(PlannerInfo *root, RelOptInfo *rel, * Populate childrel's base restriction quals from parent rel's quals, * translating them using appinfo. * - * 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. + * If any of the resulting clauses evaluate to constant false or NULL, or are + * proven always false, 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. + * + * If any resulting clauses are proven always true, just drop them. */ bool apply_child_basequals(PlannerInfo *root, RelOptInfo *parentrel, @@ -875,6 +878,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 +890,24 @@ 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..975f2d87ea 100644 --- a/src/backend/optimizer/util/plancat.c +++ b/src/backend/optimizer/util/plancat.c @@ -161,22 +161,38 @@ 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. + * + * Traditional inheritance parents might have NOT NULL constraints marked + * NO INHERIT, while their child tables do not have NOT NULL constraints. + * We should not collect NOT NULL columns for such an inheritance parent. + * Otherwise we would have problems when we have removed redundant IS NOT + * NULL restriction clauses of the parent rel, as this could cause NULL + * values from child tables to not be filtered out, or when we have reduced + * IS NULL restriction clauses of the parent rel to constant-FALSE, as this + * could cause NULL values from child tables to not be selected out. + * + * We do not have this problem with partitioned tables though. + */ + 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..b43ae951ea 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -372,11 +372,23 @@ build_simple_rel(PlannerInfo *root, int relid, RelOptInfo *parent) break; } + /* + * Save the (almost) finished struct in the query's simple_rel_array. + * + * We need to do this before we copy the parent's quals to the child, + * because when we try to determine if any quals can be proven always false + * or true, we need to find the base or otherrel relation entry in the + * query's simple_rel_array. + */ + root->simple_rel_array[relid] = rel; + /* * 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.) + * variables. If there are any resulting clauses that are constant false + * or NULL, or proven always false, 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.) For resulting clauses that + * are proven always true, we just drop them. */ if (parent) { @@ -386,16 +398,14 @@ 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. + * Some restriction clause reduced to constant FALSE or NULL, or + * was proven always false after substitution, so this child need + * not be scanned. */ 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/test/regress/expected/predicate.out b/src/test/regress/expected/predicate.out index 60bf3e37aa..329e0f0e12 100644 --- a/src/test/regress/expected/predicate.out +++ b/src/test/regress/expected/predicate.out @@ -242,3 +242,63 @@ SELECT * FROM pred_tab t1 (9 rows) DROP TABLE pred_tab; +-- +-- Tests for inheritance parents +-- +CREATE TABLE pred_parent (a int, b int); +CREATE TABLE pred_child () INHERITS (pred_parent); +ALTER TABLE ONLY pred_parent ALTER a SET NOT NULL; +-- Ensure the IS_NOT_NULL qual on pred_parent is ignored while the IS_NOT_NULL +-- qual on pred_child is not ignored +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 the IS_NULL qual on pred_parent is reduced to constant-FALSE while +-- the IS_NULL qual on pred_child is not reduced to constant-FALSE +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) + +DROP TABLE pred_child; +DROP TABLE pred_parent; +-- +-- Tests for partitioned tables +-- +CREATE TABLE pred_prt (a int, b int) PARTITION BY RANGE(a); +CREATE TABLE pred_prt_p1 PARTITION OF pred_prt FOR VALUES FROM (0) TO (250); +CREATE TABLE pred_prt_p2 PARTITION OF pred_prt FOR VALUES FROM (250) TO (500); +ALTER TABLE pred_prt_p1 ALTER b SET NOT NULL; +-- Ensure the IS_NOT_NULL qual on pred_prt_p1 is ignored while the IS_NOT_NULL +-- qual on pred_prt_p2 is not ignored +EXPLAIN (COSTS OFF) +SELECT * FROM pred_prt WHERE b IS NOT NULL; + QUERY PLAN +------------------------------------------ + Append + -> Seq Scan on pred_prt_p1 pred_prt_1 + -> Seq Scan on pred_prt_p2 pred_prt_2 + Filter: (b IS NOT NULL) +(4 rows) + +-- Ensure the IS_NULL qual on pred_prt_p1 is reduced to constant-FALSE while +-- the IS_NULL qual on pred_prt_p2 is not reduced to constant-FALSE +EXPLAIN (COSTS OFF) +SELECT * FROM pred_prt WHERE b IS NULL; + QUERY PLAN +---------------------------------- + Seq Scan on pred_prt_p2 pred_prt + Filter: (b IS NULL) +(2 rows) + +DROP TABLE pred_prt; diff --git a/src/test/regress/sql/predicate.sql b/src/test/regress/sql/predicate.sql index 563e395fde..817d1aa986 100644 --- a/src/test/regress/sql/predicate.sql +++ b/src/test/regress/sql/predicate.sql @@ -120,3 +120,45 @@ SELECT * FROM pred_tab t1 LEFT JOIN pred_tab t3 ON t2.a IS NULL OR t2.c IS NULL; DROP TABLE pred_tab; + +-- +-- Tests for inheritance parents +-- +CREATE TABLE pred_parent (a int, b int); +CREATE TABLE pred_child () INHERITS (pred_parent); + +ALTER TABLE ONLY pred_parent ALTER a SET NOT NULL; + +-- Ensure the IS_NOT_NULL qual on pred_parent is ignored while the IS_NOT_NULL +-- qual on pred_child is not ignored +EXPLAIN (COSTS OFF) +SELECT * FROM pred_parent WHERE a IS NOT NULL; + +-- Ensure the IS_NULL qual on pred_parent is reduced to constant-FALSE while +-- the IS_NULL qual on pred_child is not reduced to constant-FALSE +EXPLAIN (COSTS OFF) +SELECT * FROM pred_parent WHERE a IS NULL; + +DROP TABLE pred_child; +DROP TABLE pred_parent; + +-- +-- Tests for partitioned tables +-- +CREATE TABLE pred_prt (a int, b int) PARTITION BY RANGE(a); +CREATE TABLE pred_prt_p1 PARTITION OF pred_prt FOR VALUES FROM (0) TO (250); +CREATE TABLE pred_prt_p2 PARTITION OF pred_prt FOR VALUES FROM (250) TO (500); + +ALTER TABLE pred_prt_p1 ALTER b SET NOT NULL; + +-- Ensure the IS_NOT_NULL qual on pred_prt_p1 is ignored while the IS_NOT_NULL +-- qual on pred_prt_p2 is not ignored +EXPLAIN (COSTS OFF) +SELECT * FROM pred_prt WHERE b IS NOT NULL; + +-- Ensure the IS_NULL qual on pred_prt_p1 is reduced to constant-FALSE while +-- the IS_NULL qual on pred_prt_p2 is not reduced to constant-FALSE +EXPLAIN (COSTS OFF) +SELECT * FROM pred_prt WHERE b IS NULL; + +DROP TABLE pred_prt; -- 2.31.0