From 47aa95e951c8291239f15e400d767ea32b5be9b3 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 2 Oct 2004 22:39:49 +0000 Subject: [PATCH] =?UTF-8?q?Clean=20up=20handling=20of=20inherited-table=20?= =?UTF-8?q?update=20queries,=20per=20bug=20report=20from=20Sebastian=20B?= =?UTF-8?q?=C3=B6ck.=20=20The=20fix=20involves=20being=20more=20consistent?= =?UTF-8?q?=20about=20when=20rangetable=20entries=20are=20copied=20or=20mo?= =?UTF-8?q?dified.=20=20Someday=20we=20really=20need=20to=20fix=20this=20s?= =?UTF-8?q?tuff=20to=20not=20scribble=20on=20its=20input=20data=20structur?= =?UTF-8?q?es=20in=20the=20first=20place...?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/backend/optimizer/path/allpaths.c | 12 +----- src/backend/optimizer/plan/planner.c | 46 +++++++++++++------- src/backend/optimizer/prep/prepunion.c | 58 ++++++++++++++------------ src/backend/optimizer/util/clauses.c | 12 +++++- src/include/optimizer/prep.h | 5 +-- 5 files changed, 78 insertions(+), 55 deletions(-) diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index c7b5db7335..b01025a85d 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/optimizer/path/allpaths.c,v 1.121 2004/08/29 05:06:43 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/path/allpaths.c,v 1.122 2004/10/02 22:39:45 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -124,8 +124,7 @@ set_base_rel_pathlists(Query *root) /* RangeFunction --- generate a separate plan for it */ set_function_pathlist(root, rel, rte); } - else if ((inheritlist = expand_inherited_rtentry(root, rti, true)) - != NIL) + else if ((inheritlist = expand_inherited_rtentry(root, rti)) != NIL) { /* Relation is root of an inheritance tree, process specially */ set_inherited_rel_pathlist(root, rel, rti, rte, inheritlist); @@ -223,13 +222,6 @@ set_inherited_rel_pathlist(Query *root, RelOptInfo *rel, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("SELECT FOR UPDATE is not supported for inheritance queries"))); - /* - * The executor will check the parent table's access permissions when - * it examines the parent's inheritlist entry. There's no need to - * check twice, so turn off access check bits in the original RTE. - */ - rte->requiredPerms = 0; - /* * Initialize to compute size estimates for whole inheritance tree */ diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index d8f228f2b9..1831c1bb9c 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.175 2004/08/30 02:54:38 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.176 2004/10/02 22:39:46 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -320,8 +320,7 @@ subquery_planner(Query *parse, double tuple_fraction) * grouping_planner. */ if (parse->resultRelation && - (lst = expand_inherited_rtentry(parse, parse->resultRelation, - false)) != NIL) + (lst = expand_inherited_rtentry(parse, parse->resultRelation)) != NIL) plan = inheritance_planner(parse, lst); else plan = grouping_planner(parse, tuple_fraction); @@ -513,7 +512,6 @@ inheritance_planner(Query *parse, List *inheritlist) { int childRTindex = lfirst_int(l); Oid childOID = getrelid(childRTindex, parse->rtable); - int subrtlength; Query *subquery; Plan *subplan; @@ -526,22 +524,40 @@ inheritance_planner(Query *parse, List *inheritlist) subplans = lappend(subplans, subplan); /* - * It's possible that additional RTEs got added to the rangetable - * due to expansion of inherited source tables (see allpaths.c). - * If so, we must copy 'em back to the main parse tree's rtable. + * XXX my goodness this next bit is ugly. Really need to think about + * ways to rein in planner's habit of scribbling on its input. * - * XXX my goodness this is ugly. Really need to think about ways to - * rein in planner's habit of scribbling on its input. + * Planning of the subquery might have modified the rangetable, + * either by addition of RTEs due to expansion of inherited source + * tables, or by changes of the Query structures inside subquery + * RTEs. We have to ensure that this gets propagated back to the + * master copy. However, if we aren't done planning yet, we also + * need to ensure that subsequent calls to grouping_planner have + * virgin sub-Queries to work from. So, if we are at the last + * list entry, just copy the subquery rangetable back to the master + * copy; if we are not, then extend the master copy by adding + * whatever the subquery added. (We assume these added entries + * will go untouched by the future grouping_planner calls. We are + * also effectively assuming that sub-Queries will get planned + * identically each time, or at least that the impacts on their + * rangetables will be the same each time. Did I say this is ugly?) */ - subrtlength = list_length(subquery->rtable); - if (subrtlength > mainrtlength) + if (lnext(l) == NULL) + parse->rtable = subquery->rtable; + else { - List *subrt; + int subrtlength = list_length(subquery->rtable); - subrt = list_copy_tail(subquery->rtable, mainrtlength); - parse->rtable = list_concat(parse->rtable, subrt); - mainrtlength = subrtlength; + if (subrtlength > mainrtlength) + { + List *subrt; + + subrt = list_copy_tail(subquery->rtable, mainrtlength); + parse->rtable = list_concat(parse->rtable, subrt); + mainrtlength = subrtlength; + } } + /* Save preprocessed tlist from first rel for use in Append */ if (tlist == NIL) tlist = subplan->targetlist; diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c index 12d492f803..ba1de1b886 100644 --- a/src/backend/optimizer/prep/prepunion.c +++ b/src/backend/optimizer/prep/prepunion.c @@ -14,7 +14,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/optimizer/prep/prepunion.c,v 1.116 2004/08/29 05:06:44 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/prep/prepunion.c,v 1.117 2004/10/02 22:39:47 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -705,24 +705,23 @@ find_all_inheritors(Oid parentrel) * whole inheritance set (parent and children). * If not, return NIL. * - * When dup_parent is false, the initially given RT index is part of the - * returned list (if any). When dup_parent is true, the given RT index - * is *not* in the returned list; a duplicate RTE will be made for the - * parent table. + * Note that the original RTE is considered to represent the whole + * inheritance set. The first member of the returned list is an RTE + * for the same table, but with inh = false, to represent the parent table + * in its role as a simple member of the set. The original RT index is + * never a member of the returned list. * * A childless table is never considered to be an inheritance set; therefore * the result will never be a one-element list. It'll be either empty * or have two or more elements. * - * NOTE: after this routine executes, the specified RTE will always have - * its inh flag cleared, whether or not there were any children. This - * ensures we won't expand the same RTE twice, which would otherwise occur - * for the case of an inherited UPDATE/DELETE target relation. - * - * XXX probably should convert the result type to Relids? + * Note: there are cases in which this routine will be invoked multiple + * times on the same RTE. We will generate a separate set of child RTEs + * for each invocation. This is somewhat wasteful but seems not worth + * trying to avoid. */ List * -expand_inherited_rtentry(Query *parse, Index rti, bool dup_parent) +expand_inherited_rtentry(Query *parse, Index rti) { RangeTblEntry *rte = rt_fetch(rti, parse->rtable); Oid parentOID; @@ -734,12 +733,14 @@ expand_inherited_rtentry(Query *parse, Index rti, bool dup_parent) if (!rte->inh) return NIL; Assert(rte->rtekind == RTE_RELATION); - /* Always clear the parent's inh flag, see above comments */ - rte->inh = false; /* Fast path for common case of childless table */ parentOID = rte->relid; if (!has_subclass(parentOID)) + { + /* Clear flag to save repeated tests if called again */ + rte->inh = false; return NIL; + } /* Scan for all members of inheritance set */ inhOIDs = find_all_inheritors(parentOID); @@ -749,37 +750,42 @@ expand_inherited_rtentry(Query *parse, Index rti, bool dup_parent) * table once had a child but no longer does. */ if (list_length(inhOIDs) < 2) + { + /* Clear flag to save repeated tests if called again */ + rte->inh = false; return NIL; - /* OK, it's an inheritance set; expand it */ - if (dup_parent) - inhRTIs = NIL; - else - inhRTIs = list_make1_int(rti); /* include original RTE in result */ + } + /* OK, it's an inheritance set; expand it */ + inhRTIs = NIL; foreach(l, inhOIDs) { Oid childOID = lfirst_oid(l); RangeTblEntry *childrte; Index childRTindex; - /* parent will be in the list too; skip it if not dup requested */ - if (childOID == parentOID && !dup_parent) - continue; - /* * Build an RTE for the child, and attach to query's rangetable * list. We copy most fields of the parent's RTE, but replace - * relation real name and OID. Note that inh will be false at - * this point. + * relation OID, and set inh = false. */ childrte = copyObject(rte); childrte->relid = childOID; + childrte->inh = false; parse->rtable = lappend(parse->rtable, childrte); childRTindex = list_length(parse->rtable); - inhRTIs = lappend_int(inhRTIs, childRTindex); } + /* + * The executor will check the parent table's access permissions when + * it examines the parent's inheritlist entry. There's no need to + * check twice, so turn off access check bits in the original RTE. + * (If we are invoked more than once, extra copies of the child RTEs + * will also not cause duplicate permission checks.) + */ + rte->requiredPerms = 0; + return inhRTIs; } diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index de0e7a539c..1f848cd9bd 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.180 2004/08/29 05:06:44 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.181 2004/10/02 22:39:48 tgl Exp $ * * HISTORY * AUTHOR DATE MAJOR EVENT @@ -3254,10 +3254,20 @@ query_tree_mutator(Query *query, CHECKFLATCOPY(newrte->subquery, rte->subquery, Query); MUTATE(newrte->subquery, newrte->subquery, Query *); } + else + { + /* else, copy RT subqueries as-is */ + newrte->subquery = copyObject(rte->subquery); + } break; case RTE_JOIN: if (!(flags & QTW_IGNORE_JOINALIASES)) MUTATE(newrte->joinaliasvars, rte->joinaliasvars, List *); + else + { + /* else, copy join aliases as-is */ + newrte->joinaliasvars = copyObject(rte->joinaliasvars); + } break; case RTE_FUNCTION: MUTATE(newrte->funcexpr, rte->funcexpr, Node *); diff --git a/src/include/optimizer/prep.h b/src/include/optimizer/prep.h index 5284d224bc..9f84d29f39 100644 --- a/src/include/optimizer/prep.h +++ b/src/include/optimizer/prep.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2004, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/optimizer/prep.h,v 1.45 2004/08/29 04:13:08 momjian Exp $ + * $PostgreSQL: pgsql/src/include/optimizer/prep.h,v 1.46 2004/10/02 22:39:49 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -52,8 +52,7 @@ extern Plan *plan_set_operations(Query *parse, List **sortClauses); extern List *find_all_inheritors(Oid parentrel); -extern List *expand_inherited_rtentry(Query *parse, Index rti, - bool dup_parent); +extern List *expand_inherited_rtentry(Query *parse, Index rti); extern Node *adjust_inherited_attrs(Node *node, Index old_rt_index, Oid old_relid,