Skip to content

Commit

Permalink
Repair two constraint-exclusion corner cases triggered by proving tha…
Browse files Browse the repository at this point in the history
…t an

inheritance child of an UPDATE/DELETE target relation can be excluded by
constraints.  I had rearranged some code in set_append_rel_pathlist() to
avoid "useless" work when a child is excluded, but overdid it and left
the child with no cheapest_path entry, causing possible failure later
if the appendrel was involved in a join.  Also, it seems that the dummy
plan generated by inheritance_planner() when all branches are excluded
has to be a bit less dummy now than was required in 8.2.
Per report from Jan Wieck.  Add his test case to the regression tests.
  • Loading branch information
tglsfdc committed May 26, 2007
1 parent 604ffd2 commit cadb783
Show file tree
Hide file tree
Showing 4 changed files with 134 additions and 16 deletions.
47 changes: 32 additions & 15 deletions src/backend/optimizer/path/allpaths.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/optimizer/path/allpaths.c,v 1.163 2007/04/21 21:01:44 tgl Exp $
* $PostgreSQL: pgsql/src/backend/optimizer/path/allpaths.c,v 1.164 2007/05/26 18:23:01 tgl Exp $
*
*-------------------------------------------------------------------------
*/
Expand Down Expand Up @@ -45,6 +45,7 @@ static void set_plain_rel_pathlist(PlannerInfo *root, RelOptInfo *rel,
RangeTblEntry *rte);
static void set_append_rel_pathlist(PlannerInfo *root, RelOptInfo *rel,
Index rti, RangeTblEntry *rte);
static void set_dummy_rel_pathlist(RelOptInfo *rel);
static void set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
Index rti, RangeTblEntry *rte);
static void set_function_pathlist(PlannerInfo *root, RelOptInfo *rel,
Expand Down Expand Up @@ -198,23 +199,14 @@ set_plain_rel_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
{
/*
* If we can prove we don't need to scan the rel via constraint exclusion,
* set up a single dummy path for it. (Rather than inventing a special
* "dummy" path type, we represent this as an AppendPath with no members.)
* We only need to check for regular baserels; if it's an otherrel, CE
* was already checked in set_append_rel_pathlist().
* set up a single dummy path for it. We only need to check for regular
* baserels; if it's an otherrel, CE was already checked in
* set_append_rel_pathlist().
*/
if (rel->reloptkind == RELOPT_BASEREL &&
relation_excluded_by_constraints(rel, rte))
{
/* Set dummy size estimates --- we leave attr_widths[] as zeroes */
rel->rows = 0;
rel->width = 0;

add_path(rel, (Path *) create_append_path(rel, NIL));

/* Select cheapest path (pretty easy in this case...) */
set_cheapest(rel);

set_dummy_rel_pathlist(rel);
return;
}

Expand Down Expand Up @@ -330,7 +322,12 @@ set_append_rel_pathlist(PlannerInfo *root, RelOptInfo *rel,

if (relation_excluded_by_constraints(childrel, childRTE))
{
/* this child need not be scanned, so just disregard it */
/*
* This child need not be scanned, so we can omit it from the
* appendrel. Mark it with a dummy cheapest-path though, in
* case best_appendrel_indexscan() looks at it later.
*/
set_dummy_rel_pathlist(childrel);
continue;
}

Expand Down Expand Up @@ -425,6 +422,26 @@ set_append_rel_pathlist(PlannerInfo *root, RelOptInfo *rel,
set_cheapest(rel);
}

/*
* set_dummy_rel_pathlist
* Build a dummy path for a relation that's been excluded by constraints
*
* Rather than inventing a special "dummy" path type, we represent this as an
* AppendPath with no members.
*/
static void
set_dummy_rel_pathlist(RelOptInfo *rel)
{
/* Set dummy size estimates --- we leave attr_widths[] as zeroes */
rel->rows = 0;
rel->width = 0;

add_path(rel, (Path *) create_append_path(rel, NIL));

/* Select cheapest path (pretty easy in this case...) */
set_cheapest(rel);
}

/* quick-and-dirty test to see if any joining is needed */
static bool
has_multiple_baserels(PlannerInfo *root)
Expand Down
7 changes: 6 additions & 1 deletion src/backend/optimizer/plan/planner.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.220 2007/05/25 17:54:25 tgl Exp $
* $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.221 2007/05/26 18:23:01 tgl Exp $
*
*-------------------------------------------------------------------------
*/
Expand Down Expand Up @@ -669,11 +669,16 @@ inheritance_planner(PlannerInfo *root)
* If we managed to exclude every child rel, return a dummy plan
*/
if (subplans == NIL)
{
root->resultRelations = list_make1_int(parentRTindex);
/* although dummy, it must have a valid tlist for executor */
tlist = preprocess_targetlist(root, parse->targetList);
return (Plan *) make_result(root,
tlist,
(Node *) list_make1(makeBoolConst(false,
false)),
NULL);
}

/*
* Planning might have modified the rangetable, due to changes of the
Expand Down
58 changes: 58 additions & 0 deletions src/test/regress/expected/rules.out
Original file line number Diff line number Diff line change
Expand Up @@ -1493,3 +1493,61 @@ select * from id_ordered;

set client_min_messages to warning; -- suppress cascade notices
drop table id cascade;
reset client_min_messages;
--
-- check corner case where an entirely-dummy subplan is created by
-- constraint exclusion
--
create temp table t1 (a integer primary key);
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "t1_pkey" for table "t1"
create temp table t1_1 (check (a >= 0 and a < 10)) inherits (t1);
create temp table t1_2 (check (a >= 10 and a < 20)) inherits (t1);
create rule t1_ins_1 as on insert to t1
where new.a >= 0 and new.a < 10
do instead
insert into t1_1 values (new.a);
create rule t1_ins_2 as on insert to t1
where new.a >= 10 and new.a < 20
do instead
insert into t1_2 values (new.a);
create rule t1_upd_1 as on update to t1
where old.a >= 0 and old.a < 10
do instead
update t1_1 set a = new.a where a = old.a;
create rule t1_upd_2 as on update to t1
where old.a >= 10 and old.a < 20
do instead
update t1_2 set a = new.a where a = old.a;
set constraint_exclusion = on;
insert into t1 select * from generate_series(5,19,1) g;
update t1 set a = 4 where a = 5;
select * from only t1;
a
---
(0 rows)

select * from only t1_1;
a
---
6
7
8
9
4
(5 rows)

select * from only t1_2;
a
----
10
11
12
13
14
15
16
17
18
19
(10 rows)

38 changes: 38 additions & 0 deletions src/test/regress/sql/rules.sql
Original file line number Diff line number Diff line change
Expand Up @@ -881,3 +881,41 @@ select * from id_ordered;

set client_min_messages to warning; -- suppress cascade notices
drop table id cascade;
reset client_min_messages;

--
-- check corner case where an entirely-dummy subplan is created by
-- constraint exclusion
--

create temp table t1 (a integer primary key);

create temp table t1_1 (check (a >= 0 and a < 10)) inherits (t1);
create temp table t1_2 (check (a >= 10 and a < 20)) inherits (t1);

create rule t1_ins_1 as on insert to t1
where new.a >= 0 and new.a < 10
do instead
insert into t1_1 values (new.a);
create rule t1_ins_2 as on insert to t1
where new.a >= 10 and new.a < 20
do instead
insert into t1_2 values (new.a);

create rule t1_upd_1 as on update to t1
where old.a >= 0 and old.a < 10
do instead
update t1_1 set a = new.a where a = old.a;
create rule t1_upd_2 as on update to t1
where old.a >= 10 and old.a < 20
do instead
update t1_2 set a = new.a where a = old.a;

set constraint_exclusion = on;

insert into t1 select * from generate_series(5,19,1) g;
update t1 set a = 4 where a = 5;

select * from only t1;
select * from only t1_1;
select * from only t1_2;

0 comments on commit cadb783

Please sign in to comment.