Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 3af7040

Browse filesBrowse files
david-rowleydavid-rowley
committed
Fix IS [NOT] NULL qual optimization for inheritance tables
b262ad4 added code to have the planner remove redundant IS NOT NULL quals and eliminate needless scans for IS NULL quals on tables where the qual's column has a NOT NULL constraint. That commit failed to consider that an inheritance parent table could have differing NOT NULL constraints between the parent and the child. This caused issues as if we eliminated a qual on the parent, when applying the quals to child tables in apply_child_basequals(), the qual might not have been added to the parent's baserestrictinfo. Here we fix this by not applying the optimization to remove redundant quals to RelOptInfos belonging to inheritance parents and applying the optimization again in apply_child_basequals(). Effectively, this means that the parent and child are considered independently as the parent has both an inh=true and inh=false RTE and we still apply the optimization to the RelOptInfo corresponding to the inh=false RTE. We're able to still apply the optimization in add_base_clause_to_rel() for partitioned tables as the NULLability of partitions must match that of their parent. And, if we ever expand restriction_is_always_false() and restriction_is_always_true() to handle partition constraints then we can apply the same logic as, even in multi-level partitioned tables, there's no way to route values to a partition when the qual does not match the partition qual of the partitioned table's parent partition. The same is true for CHECK constraints as those must also match between arent partitioned tables and their partitions. Author: Richard Guo, David Rowley Discussion: https://postgr.es/m/CAMbWs4930gQSZmjR7aANzEapdy61gCg6z8dT-kAEYD0sYWKPdQ@mail.gmail.com
1 parent 6d4f062 commit 3af7040
Copy full SHA for 3af7040

File tree

Expand file treeCollapse file tree

7 files changed

+182
-54
lines changed
Filter options
Expand file treeCollapse file tree

7 files changed

+182
-54
lines changed

‎src/backend/optimizer/plan/initsplan.c

Copy file name to clipboardExpand all lines: src/backend/optimizer/plan/initsplan.c
+40-20Lines changed: 40 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include "parser/analyze.h"
3232
#include "rewrite/rewriteManip.h"
3333
#include "utils/lsyscache.h"
34+
#include "utils/rel.h"
3435
#include "utils/typcache.h"
3536

3637
/* These parameters are set by GUC */
@@ -2629,32 +2630,51 @@ add_base_clause_to_rel(PlannerInfo *root, Index relid,
26292630
RestrictInfo *restrictinfo)
26302631
{
26312632
RelOptInfo *rel = find_base_rel(root, relid);
2633+
RangeTblEntry *rte = root->simple_rte_array[relid];
26322634

26332635
Assert(bms_membership(restrictinfo->required_relids) == BMS_SINGLETON);
26342636

2635-
/* Don't add the clause if it is always true */
2636-
if (restriction_is_always_true(root, restrictinfo))
2637-
return;
2638-
26392637
/*
2640-
* Substitute the origin qual with constant-FALSE if it is provably always
2641-
* false. Note that we keep the same rinfo_serial.
2638+
* For inheritance parent tables, we must always record the RestrictInfo
2639+
* in baserestrictinfo as is. If we were to transform or skip adding it,
2640+
* then the original wouldn't be available in apply_child_basequals. Since
2641+
* there are two RangeTblEntries for inheritance parents, one with
2642+
* inh==true and the other with inh==false, we're still able to apply this
2643+
* optimization to the inh==false one. The inh==true one is what
2644+
* apply_child_basequals() sees, whereas the inh==false one is what's used
2645+
* for the scan node in the final plan.
2646+
*
2647+
* We make an exception to this is for partitioned tables. For these, we
2648+
* always apply the constant-TRUE and constant-FALSE transformations. A
2649+
* qual which is either of these for a partitioned table must also be that
2650+
* for all of its child partitions.
26422651
*/
2643-
if (restriction_is_always_false(root, restrictinfo))
2652+
if (!rte->inh || rte->relkind == RELKIND_PARTITIONED_TABLE)
26442653
{
2645-
int save_rinfo_serial = restrictinfo->rinfo_serial;
2646-
2647-
restrictinfo = make_restrictinfo(root,
2648-
(Expr *) makeBoolConst(false, false),
2649-
restrictinfo->is_pushed_down,
2650-
restrictinfo->has_clone,
2651-
restrictinfo->is_clone,
2652-
restrictinfo->pseudoconstant,
2653-
0, /* security_level */
2654-
restrictinfo->required_relids,
2655-
restrictinfo->incompatible_relids,
2656-
restrictinfo->outer_relids);
2657-
restrictinfo->rinfo_serial = save_rinfo_serial;
2654+
/* Don't add the clause if it is always true */
2655+
if (restriction_is_always_true(root, restrictinfo))
2656+
return;
2657+
2658+
/*
2659+
* Substitute the origin qual with constant-FALSE if it is provably
2660+
* always false. Note that we keep the same rinfo_serial.
2661+
*/
2662+
if (restriction_is_always_false(root, restrictinfo))
2663+
{
2664+
int save_rinfo_serial = restrictinfo->rinfo_serial;
2665+
2666+
restrictinfo = make_restrictinfo(root,
2667+
(Expr *) makeBoolConst(false, false),
2668+
restrictinfo->is_pushed_down,
2669+
restrictinfo->has_clone,
2670+
restrictinfo->is_clone,
2671+
restrictinfo->pseudoconstant,
2672+
0, /* security_level */
2673+
restrictinfo->required_relids,
2674+
restrictinfo->incompatible_relids,
2675+
restrictinfo->outer_relids);
2676+
restrictinfo->rinfo_serial = save_rinfo_serial;
2677+
}
26582678
}
26592679

26602680
/* Add clause to rel's restriction list */

‎src/backend/optimizer/util/inherit.c

Copy file name to clipboardExpand all lines: src/backend/optimizer/util/inherit.c
+22-11Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -822,11 +822,13 @@ expand_appendrel_subquery(PlannerInfo *root, RelOptInfo *rel,
822822
/*
823823
* apply_child_basequals
824824
* Populate childrel's base restriction quals from parent rel's quals,
825-
* translating them using appinfo.
825+
* translating Vars using appinfo and re-checking for quals which are
826+
* constant-TRUE or constant-FALSE when applied to this child relation.
826827
*
827828
* If any of the resulting clauses evaluate to constant false or NULL, we
828829
* return false and don't apply any quals. Caller should mark the relation as
829-
* a dummy rel in this case, since it doesn't need to be scanned.
830+
* a dummy rel in this case, since it doesn't need to be scanned. Constant
831+
* true quals are ignored.
830832
*/
831833
bool
832834
apply_child_basequals(PlannerInfo *root, RelOptInfo *parentrel,
@@ -875,6 +877,7 @@ apply_child_basequals(PlannerInfo *root, RelOptInfo *parentrel,
875877
{
876878
Node *onecq = (Node *) lfirst(lc2);
877879
bool pseudoconstant;
880+
RestrictInfo *childrinfo;
878881

879882
/* check for pseudoconstant (no Vars or volatile functions) */
880883
pseudoconstant =
@@ -886,15 +889,23 @@ apply_child_basequals(PlannerInfo *root, RelOptInfo *parentrel,
886889
root->hasPseudoConstantQuals = true;
887890
}
888891
/* reconstitute RestrictInfo with appropriate properties */
889-
childquals = lappend(childquals,
890-
make_restrictinfo(root,
891-
(Expr *) onecq,
892-
rinfo->is_pushed_down,
893-
rinfo->has_clone,
894-
rinfo->is_clone,
895-
pseudoconstant,
896-
rinfo->security_level,
897-
NULL, NULL, NULL));
892+
childrinfo = make_restrictinfo(root,
893+
(Expr *) onecq,
894+
rinfo->is_pushed_down,
895+
rinfo->has_clone,
896+
rinfo->is_clone,
897+
pseudoconstant,
898+
rinfo->security_level,
899+
NULL, NULL, NULL);
900+
901+
/* Restriction is proven always false */
902+
if (restriction_is_always_false(root, childrinfo))
903+
return false;
904+
/* Restriction is proven always true, so drop it */
905+
if (restriction_is_always_true(root, childrinfo))
906+
continue;
907+
908+
childquals = lappend(childquals, childrinfo);
898909
/* track minimum security level among child quals */
899910
cq_min_security = Min(cq_min_security, rinfo->security_level);
900911
}

‎src/backend/optimizer/util/plancat.c

Copy file name to clipboardExpand all lines: src/backend/optimizer/util/plancat.c
+24-13Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -161,22 +161,33 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
161161
rel->attr_widths = (int32 *)
162162
palloc0((rel->max_attr - rel->min_attr + 1) * sizeof(int32));
163163

164-
/* record which columns are defined as NOT NULL */
165-
for (int i = 0; i < relation->rd_att->natts; i++)
164+
/*
165+
* Record which columns are defined as NOT NULL. We leave this
166+
* unpopulated for non-partitioned inheritance parent relations as it's
167+
* ambiguous as to what it means. Some child tables may have a NOT NULL
168+
* constraint for a column while others may not. We could work harder and
169+
* build a unioned set of all child relations notnullattnums, but there's
170+
* currently no need. The RelOptInfo corresponding to the !inh
171+
* RangeTblEntry does get populated.
172+
*/
173+
if (!inhparent || relation->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
166174
{
167-
FormData_pg_attribute *attr = &relation->rd_att->attrs[i];
168-
169-
if (attr->attnotnull)
175+
for (int i = 0; i < relation->rd_att->natts; i++)
170176
{
171-
rel->notnullattnums = bms_add_member(rel->notnullattnums,
172-
attr->attnum);
177+
FormData_pg_attribute *attr = &relation->rd_att->attrs[i];
173178

174-
/*
175-
* Per RemoveAttributeById(), dropped columns will have their
176-
* attnotnull unset, so we needn't check for dropped columns in
177-
* the above condition.
178-
*/
179-
Assert(!attr->attisdropped);
179+
if (attr->attnotnull)
180+
{
181+
rel->notnullattnums = bms_add_member(rel->notnullattnums,
182+
attr->attnum);
183+
184+
/*
185+
* Per RemoveAttributeById(), dropped columns will have their
186+
* attnotnull unset, so we needn't check for dropped columns
187+
* in the above condition.
188+
*/
189+
Assert(!attr->attisdropped);
190+
}
180191
}
181192
}
182193

‎src/backend/optimizer/util/relnode.c

Copy file name to clipboardExpand all lines: src/backend/optimizer/util/relnode.c
+16-9Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -373,10 +373,20 @@ build_simple_rel(PlannerInfo *root, int relid, RelOptInfo *parent)
373373
}
374374

375375
/*
376-
* Copy the parent's quals to the child, with appropriate substitution of
377-
* variables. If any constant false or NULL clauses turn up, we can mark
378-
* the child as dummy right away. (We must do this immediately so that
379-
* pruning works correctly when recursing in expand_partitioned_rtentry.)
376+
* We must apply the partially filled in RelOptInfo before calling
377+
* apply_child_basequals due to some transformations within that function
378+
* which require the RelOptInfo to be available in the simple_rel_array.
379+
*/
380+
root->simple_rel_array[relid] = rel;
381+
382+
/*
383+
* Apply the parent's quals to the child, with appropriate substitution of
384+
* variables. If the resulting clause is constant-FALSE or NULL after
385+
* applying transformations, apply_child_basequals returns false to
386+
* indicate that scanning this relation won't yield any rows. In this
387+
* case, we mark the child as dummy right away. (We must do this
388+
* immediately so that pruning works correctly when recursing in
389+
* expand_partitioned_rtentry.)
380390
*/
381391
if (parent)
382392
{
@@ -386,16 +396,13 @@ build_simple_rel(PlannerInfo *root, int relid, RelOptInfo *parent)
386396
if (!apply_child_basequals(root, parent, rel, rte, appinfo))
387397
{
388398
/*
389-
* Some restriction clause reduced to constant FALSE or NULL after
390-
* substitution, so this child need not be scanned.
399+
* Restriction clause reduced to constant FALSE or NULL. Mark as
400+
* dummy so we won't scan this relation.
391401
*/
392402
mark_dummy_rel(rel);
393403
}
394404
}
395405

396-
/* Save the finished struct in the query's simple_rel_array */
397-
root->simple_rel_array[relid] = rel;
398-
399406
return rel;
400407
}
401408

‎src/include/nodes/pathnodes.h

Copy file name to clipboardExpand all lines: src/include/nodes/pathnodes.h
+5-1Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -918,7 +918,11 @@ typedef struct RelOptInfo
918918
Relids *attr_needed pg_node_attr(read_write_ignore);
919919
/* array indexed [min_attr .. max_attr] */
920920
int32 *attr_widths pg_node_attr(read_write_ignore);
921-
/* zero-based set containing attnums of NOT NULL columns */
921+
922+
/*
923+
* Zero-based set containing attnums of NOT NULL columns. Not populated
924+
* for rels corresponding to non-partitioned inh==true RTEs.
925+
*/
922926
Bitmapset *notnullattnums;
923927
/* relids of outer joins that can null this baserel */
924928
Relids nulling_relids;

‎src/test/regress/expected/predicate.out

Copy file name to clipboardExpand all lines: src/test/regress/expected/predicate.out
+48Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,3 +242,51 @@ SELECT * FROM pred_tab t1
242242
(9 rows)
243243

244244
DROP TABLE pred_tab;
245+
-- Validate we handle IS NULL and IS NOT NULL quals correctly with inheritance
246+
-- parents.
247+
CREATE TABLE pred_parent (a int);
248+
CREATE TABLE pred_child () INHERITS (pred_parent);
249+
ALTER TABLE ONLY pred_parent ALTER a SET NOT NULL;
250+
-- Ensure that the scan on pred_child contains the IS NOT NULL qual.
251+
EXPLAIN (COSTS OFF)
252+
SELECT * FROM pred_parent WHERE a IS NOT NULL;
253+
QUERY PLAN
254+
---------------------------------------------
255+
Append
256+
-> Seq Scan on pred_parent pred_parent_1
257+
-> Seq Scan on pred_child pred_parent_2
258+
Filter: (a IS NOT NULL)
259+
(4 rows)
260+
261+
-- Ensure we only scan pred_child and not pred_parent
262+
EXPLAIN (COSTS OFF)
263+
SELECT * FROM pred_parent WHERE a IS NULL;
264+
QUERY PLAN
265+
------------------------------------
266+
Seq Scan on pred_child pred_parent
267+
Filter: (a IS NULL)
268+
(2 rows)
269+
270+
ALTER TABLE pred_parent ALTER a DROP NOT NULL;
271+
ALTER TABLE pred_child ALTER a SET NOT NULL;
272+
-- Ensure the IS NOT NULL qual is removed from the pred_child scan.
273+
EXPLAIN (COSTS OFF)
274+
SELECT * FROM pred_parent WHERE a IS NOT NULL;
275+
QUERY PLAN
276+
---------------------------------------------
277+
Append
278+
-> Seq Scan on pred_parent pred_parent_1
279+
Filter: (a IS NOT NULL)
280+
-> Seq Scan on pred_child pred_parent_2
281+
(4 rows)
282+
283+
-- Ensure we only scan pred_parent and not pred_child
284+
EXPLAIN (COSTS OFF)
285+
SELECT * FROM pred_parent WHERE a IS NULL;
286+
QUERY PLAN
287+
-------------------------
288+
Seq Scan on pred_parent
289+
Filter: (a IS NULL)
290+
(2 rows)
291+
292+
DROP TABLE pred_parent, pred_child;

‎src/test/regress/sql/predicate.sql

Copy file name to clipboardExpand all lines: src/test/regress/sql/predicate.sql
+27Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,3 +120,30 @@ SELECT * FROM pred_tab t1
120120
LEFT JOIN pred_tab t3 ON t2.a IS NULL OR t2.c IS NULL;
121121

122122
DROP TABLE pred_tab;
123+
124+
-- Validate we handle IS NULL and IS NOT NULL quals correctly with inheritance
125+
-- parents.
126+
CREATE TABLE pred_parent (a int);
127+
CREATE TABLE pred_child () INHERITS (pred_parent);
128+
ALTER TABLE ONLY pred_parent ALTER a SET NOT NULL;
129+
130+
-- Ensure that the scan on pred_child contains the IS NOT NULL qual.
131+
EXPLAIN (COSTS OFF)
132+
SELECT * FROM pred_parent WHERE a IS NOT NULL;
133+
134+
-- Ensure we only scan pred_child and not pred_parent
135+
EXPLAIN (COSTS OFF)
136+
SELECT * FROM pred_parent WHERE a IS NULL;
137+
138+
ALTER TABLE pred_parent ALTER a DROP NOT NULL;
139+
ALTER TABLE pred_child ALTER a SET NOT NULL;
140+
141+
-- Ensure the IS NOT NULL qual is removed from the pred_child scan.
142+
EXPLAIN (COSTS OFF)
143+
SELECT * FROM pred_parent WHERE a IS NOT NULL;
144+
145+
-- Ensure we only scan pred_parent and not pred_child
146+
EXPLAIN (COSTS OFF)
147+
SELECT * FROM pred_parent WHERE a IS NULL;
148+
149+
DROP TABLE pred_parent, pred_child;

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.