Skip to content

Commit cadb783

Browse files
tglsfdctglsfdc
committed
Repair two constraint-exclusion corner cases triggered by proving that 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.
1 parent 604ffd2 commit cadb783

File tree

4 files changed

+134
-16
lines changed

4 files changed

+134
-16
lines changed

‎src/backend/optimizer/path/allpaths.c

+32-15
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/optimizer/path/allpaths.c,v 1.163 2007/04/21 21:01:44 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/optimizer/path/allpaths.c,v 1.164 2007/05/26 18:23:01 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -45,6 +45,7 @@ static void set_plain_rel_pathlist(PlannerInfo *root, RelOptInfo *rel,
4545
RangeTblEntry *rte);
4646
static void set_append_rel_pathlist(PlannerInfo *root, RelOptInfo *rel,
4747
Index rti, RangeTblEntry *rte);
48+
static void set_dummy_rel_pathlist(RelOptInfo *rel);
4849
static void set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
4950
Index rti, RangeTblEntry *rte);
5051
static void set_function_pathlist(PlannerInfo *root, RelOptInfo *rel,
@@ -198,23 +199,14 @@ set_plain_rel_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
198199
{
199200
/*
200201
* If we can prove we don't need to scan the rel via constraint exclusion,
201-
* set up a single dummy path for it. (Rather than inventing a special
202-
* "dummy" path type, we represent this as an AppendPath with no members.)
203-
* We only need to check for regular baserels; if it's an otherrel, CE
204-
* was already checked in set_append_rel_pathlist().
202+
* set up a single dummy path for it. We only need to check for regular
203+
* baserels; if it's an otherrel, CE was already checked in
204+
* set_append_rel_pathlist().
205205
*/
206206
if (rel->reloptkind == RELOPT_BASEREL &&
207207
relation_excluded_by_constraints(rel, rte))
208208
{
209-
/* Set dummy size estimates --- we leave attr_widths[] as zeroes */
210-
rel->rows = 0;
211-
rel->width = 0;
212-
213-
add_path(rel, (Path *) create_append_path(rel, NIL));
214-
215-
/* Select cheapest path (pretty easy in this case...) */
216-
set_cheapest(rel);
217-
209+
set_dummy_rel_pathlist(rel);
218210
return;
219211
}
220212

@@ -330,7 +322,12 @@ set_append_rel_pathlist(PlannerInfo *root, RelOptInfo *rel,
330322

331323
if (relation_excluded_by_constraints(childrel, childRTE))
332324
{
333-
/* this child need not be scanned, so just disregard it */
325+
/*
326+
* This child need not be scanned, so we can omit it from the
327+
* appendrel. Mark it with a dummy cheapest-path though, in
328+
* case best_appendrel_indexscan() looks at it later.
329+
*/
330+
set_dummy_rel_pathlist(childrel);
334331
continue;
335332
}
336333

@@ -425,6 +422,26 @@ set_append_rel_pathlist(PlannerInfo *root, RelOptInfo *rel,
425422
set_cheapest(rel);
426423
}
427424

425+
/*
426+
* set_dummy_rel_pathlist
427+
* Build a dummy path for a relation that's been excluded by constraints
428+
*
429+
* Rather than inventing a special "dummy" path type, we represent this as an
430+
* AppendPath with no members.
431+
*/
432+
static void
433+
set_dummy_rel_pathlist(RelOptInfo *rel)
434+
{
435+
/* Set dummy size estimates --- we leave attr_widths[] as zeroes */
436+
rel->rows = 0;
437+
rel->width = 0;
438+
439+
add_path(rel, (Path *) create_append_path(rel, NIL));
440+
441+
/* Select cheapest path (pretty easy in this case...) */
442+
set_cheapest(rel);
443+
}
444+
428445
/* quick-and-dirty test to see if any joining is needed */
429446
static bool
430447
has_multiple_baserels(PlannerInfo *root)

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

+6-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.220 2007/05/25 17:54:25 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.221 2007/05/26 18:23:01 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -669,11 +669,16 @@ inheritance_planner(PlannerInfo *root)
669669
* If we managed to exclude every child rel, return a dummy plan
670670
*/
671671
if (subplans == NIL)
672+
{
673+
root->resultRelations = list_make1_int(parentRTindex);
674+
/* although dummy, it must have a valid tlist for executor */
675+
tlist = preprocess_targetlist(root, parse->targetList);
672676
return (Plan *) make_result(root,
673677
tlist,
674678
(Node *) list_make1(makeBoolConst(false,
675679
false)),
676680
NULL);
681+
}
677682

678683
/*
679684
* Planning might have modified the rangetable, due to changes of the

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

+58
Original file line numberDiff line numberDiff line change
@@ -1493,3 +1493,61 @@ select * from id_ordered;
14931493

14941494
set client_min_messages to warning; -- suppress cascade notices
14951495
drop table id cascade;
1496+
reset client_min_messages;
1497+
--
1498+
-- check corner case where an entirely-dummy subplan is created by
1499+
-- constraint exclusion
1500+
--
1501+
create temp table t1 (a integer primary key);
1502+
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "t1_pkey" for table "t1"
1503+
create temp table t1_1 (check (a >= 0 and a < 10)) inherits (t1);
1504+
create temp table t1_2 (check (a >= 10 and a < 20)) inherits (t1);
1505+
create rule t1_ins_1 as on insert to t1
1506+
where new.a >= 0 and new.a < 10
1507+
do instead
1508+
insert into t1_1 values (new.a);
1509+
create rule t1_ins_2 as on insert to t1
1510+
where new.a >= 10 and new.a < 20
1511+
do instead
1512+
insert into t1_2 values (new.a);
1513+
create rule t1_upd_1 as on update to t1
1514+
where old.a >= 0 and old.a < 10
1515+
do instead
1516+
update t1_1 set a = new.a where a = old.a;
1517+
create rule t1_upd_2 as on update to t1
1518+
where old.a >= 10 and old.a < 20
1519+
do instead
1520+
update t1_2 set a = new.a where a = old.a;
1521+
set constraint_exclusion = on;
1522+
insert into t1 select * from generate_series(5,19,1) g;
1523+
update t1 set a = 4 where a = 5;
1524+
select * from only t1;
1525+
a
1526+
---
1527+
(0 rows)
1528+
1529+
select * from only t1_1;
1530+
a
1531+
---
1532+
6
1533+
7
1534+
8
1535+
9
1536+
4
1537+
(5 rows)
1538+
1539+
select * from only t1_2;
1540+
a
1541+
----
1542+
10
1543+
11
1544+
12
1545+
13
1546+
14
1547+
15
1548+
16
1549+
17
1550+
18
1551+
19
1552+
(10 rows)
1553+

‎src/test/regress/sql/rules.sql

+38
Original file line numberDiff line numberDiff line change
@@ -881,3 +881,41 @@ select * from id_ordered;
881881

882882
set client_min_messages to warning; -- suppress cascade notices
883883
drop table id cascade;
884+
reset client_min_messages;
885+
886+
--
887+
-- check corner case where an entirely-dummy subplan is created by
888+
-- constraint exclusion
889+
--
890+
891+
create temp table t1 (a integer primary key);
892+
893+
create temp table t1_1 (check (a >= 0 and a < 10)) inherits (t1);
894+
create temp table t1_2 (check (a >= 10 and a < 20)) inherits (t1);
895+
896+
create rule t1_ins_1 as on insert to t1
897+
where new.a >= 0 and new.a < 10
898+
do instead
899+
insert into t1_1 values (new.a);
900+
create rule t1_ins_2 as on insert to t1
901+
where new.a >= 10 and new.a < 20
902+
do instead
903+
insert into t1_2 values (new.a);
904+
905+
create rule t1_upd_1 as on update to t1
906+
where old.a >= 0 and old.a < 10
907+
do instead
908+
update t1_1 set a = new.a where a = old.a;
909+
create rule t1_upd_2 as on update to t1
910+
where old.a >= 10 and old.a < 20
911+
do instead
912+
update t1_2 set a = new.a where a = old.a;
913+
914+
set constraint_exclusion = on;
915+
916+
insert into t1 select * from generate_series(5,19,1) g;
917+
update t1 set a = 4 where a = 5;
918+
919+
select * from only t1;
920+
select * from only t1_1;
921+
select * from only t1_2;

0 commit comments

Comments
 (0)