]> granicus.if.org Git - postgresql/commitdiff
Get rid of adjust_appendrel_attr_needed(), which has been broken ever since
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 11 Nov 2008 18:13:32 +0000 (18:13 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 11 Nov 2008 18:13:32 +0000 (18:13 +0000)
we extended the appendrel mechanism to support UNION ALL optimization.  The
reason nobody noticed was that we are not actually using attr_needed data for
appendrel children; hence it seems more reasonable to rip it out than fix it.
Back-patch to 8.2 because an Assert failure is possible in corner cases.
Per examination of an example from Jim Nasby.

In HEAD, also get rid of AppendRelInfo.col_mappings, which is quite inadequate
to represent UNION ALL situations; depend entirely on translated_vars instead.

src/backend/nodes/copyfuncs.c
src/backend/nodes/equalfuncs.c
src/backend/nodes/outfuncs.c
src/backend/optimizer/path/allpaths.c
src/backend/optimizer/prep/prepjointree.c
src/backend/optimizer/prep/prepunion.c
src/include/nodes/relation.h
src/include/optimizer/prep.h

index d56075e6bbfab7090eebae52d1deb67214f03fcf..b26fb827aa2a10c011c7b6c5f1cb440cb887baa1 100644 (file)
@@ -15,7 +15,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.410 2008/10/31 08:39:20 heikki Exp $
+ *       $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.411 2008/11/11 18:13:32 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1640,7 +1640,6 @@ _copyAppendRelInfo(AppendRelInfo *from)
        COPY_SCALAR_FIELD(child_relid);
        COPY_SCALAR_FIELD(parent_reltype);
        COPY_SCALAR_FIELD(child_reltype);
-       COPY_NODE_FIELD(col_mappings);
        COPY_NODE_FIELD(translated_vars);
        COPY_SCALAR_FIELD(parent_reloid);
 
index e17aff89228ce805856bb9784b9514619d79f502..01cb7d94c694a46d949a75458f6aedafd07d10e8 100644 (file)
@@ -22,7 +22,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.335 2008/10/31 08:39:20 heikki Exp $
+ *       $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.336 2008/11/11 18:13:32 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -806,7 +806,6 @@ _equalAppendRelInfo(AppendRelInfo *a, AppendRelInfo *b)
        COMPARE_SCALAR_FIELD(child_relid);
        COMPARE_SCALAR_FIELD(parent_reltype);
        COMPARE_SCALAR_FIELD(child_reltype);
-       COMPARE_NODE_FIELD(col_mappings);
        COMPARE_NODE_FIELD(translated_vars);
        COMPARE_SCALAR_FIELD(parent_reloid);
 
index d1130760f9e94b03d08c5487e9f64ac940656d9c..c8febd273bae1d31807c173765216b3e68c547f8 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.343 2008/10/21 20:42:52 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.344 2008/11/11 18:13:32 tgl Exp $
  *
  * NOTES
  *       Every node type that can appear in stored rules' parsetrees *must*
@@ -1628,7 +1628,6 @@ _outAppendRelInfo(StringInfo str, AppendRelInfo *node)
        WRITE_UINT_FIELD(child_relid);
        WRITE_OID_FIELD(parent_reltype);
        WRITE_OID_FIELD(child_reltype);
-       WRITE_NODE_FIELD(col_mappings);
        WRITE_NODE_FIELD(translated_vars);
        WRITE_OID_FIELD(parent_reloid);
 }
index 7d6a3b8d6b58a1c053fb376516f68d4d31d3799a..4d868570bcbf00780c9cf5cb3c8ccb98616e3c4f 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/optimizer/path/allpaths.c,v 1.175 2008/10/21 20:42:52 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/optimizer/path/allpaths.c,v 1.176 2008/11/11 18:13:32 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -382,14 +382,12 @@ set_append_rel_pathlist(PlannerInfo *root, RelOptInfo *rel,
                }
 
                /*
-                * Copy the parent's attr_needed data as well, with appropriate
-                * adjustment of relids and attribute numbers.
+                * Note: we could compute appropriate attr_needed data for the
+                * child's variables, by transforming the parent's attr_needed
+                * through the translated_vars mapping.  However, currently there's
+                * no need because attr_needed is only examined for base relations
+                * not otherrels.  So we just leave the child's attr_needed empty.
                 */
-               pfree(childrel->attr_needed);
-               childrel->attr_needed =
-                       adjust_appendrel_attr_needed(rel, appinfo,
-                                                                                childrel->min_attr,
-                                                                                childrel->max_attr);
 
                /*
                 * Compute the child's access paths, and add the cheapest one to the
index b15a0e5dd4000f1faff2398bc6fd8806dd7caffc..b5882a5771a1f4d9caadaf938d301e0ccf9bd43e 100644 (file)
@@ -16,7 +16,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/optimizer/prep/prepjointree.c,v 1.58 2008/10/22 20:17:52 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/optimizer/prep/prepjointree.c,v 1.59 2008/11/11 18:13:32 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -54,9 +54,8 @@ static Node *pull_up_simple_union_all(PlannerInfo *root, Node *jtnode,
 static void pull_up_union_leaf_queries(Node *setOp, PlannerInfo *root,
                                                  int parentRTindex, Query *setOpQuery,
                                                  int childRToffset);
-static void make_setop_translation_lists(Query *query,
-                                                        Index newvarno,
-                                                        List **col_mappings, List **translated_vars);
+static void make_setop_translation_list(Query *query, Index newvarno,
+                                                        List **translated_vars);
 static bool is_simple_subquery(Query *subquery);
 static bool is_simple_union_all(Query *subquery);
 static bool is_simple_union_all_recurse(Node *setOp, Query *setOpQuery,
@@ -839,9 +838,8 @@ pull_up_union_leaf_queries(Node *setOp, PlannerInfo *root, int parentRTindex,
                appinfo->child_relid = childRTindex;
                appinfo->parent_reltype = InvalidOid;
                appinfo->child_reltype = InvalidOid;
-               make_setop_translation_lists(setOpQuery, childRTindex,
-                                                                        &appinfo->col_mappings,
-                                                                        &appinfo->translated_vars);
+               make_setop_translation_list(setOpQuery, childRTindex,
+                                                                       &appinfo->translated_vars);
                appinfo->parent_reloid = InvalidOid;
                root->append_rel_list = lappend(root->append_rel_list, appinfo);
 
@@ -874,17 +872,16 @@ pull_up_union_leaf_queries(Node *setOp, PlannerInfo *root, int parentRTindex,
 }
 
 /*
- * make_setop_translation_lists
- *       Build the lists of translations from parent Vars to child Vars for
- *       a UNION ALL member.  We need both a column number mapping list
- *       and a list of Vars representing the child columns.
+ * make_setop_translation_list
+ *       Build the list of translations from parent Vars to child Vars for
+ *       a UNION ALL member.  (At this point it's just a simple list of
+ *       referencing Vars, but if we succeed in pulling up the member
+ *       subquery, the Vars will get replaced by pulled-up expressions.)
  */
 static void
-make_setop_translation_lists(Query *query,
-                                                        Index newvarno,
-                                                        List **col_mappings, List **translated_vars)
+make_setop_translation_list(Query *query, Index newvarno,
+                                                       List **translated_vars)
 {
-       List       *numbers = NIL;
        List       *vars = NIL;
        ListCell   *l;
 
@@ -895,7 +892,6 @@ make_setop_translation_lists(Query *query,
                if (tle->resjunk)
                        continue;
 
-               numbers = lappend_int(numbers, tle->resno);
                vars = lappend(vars, makeVar(newvarno,
                                                                         tle->resno,
                                                                         exprType((Node *) tle->expr),
@@ -903,7 +899,6 @@ make_setop_translation_lists(Query *query,
                                                                         0));
        }
 
-       *col_mappings = numbers;
        *translated_vars = vars;
 }
 
index dd7f2f28e0adfdc32b6d3b0c9fd9f054a5528ed3..c2113cb2669a2370862f2e87aa436b4d3e642475 100644 (file)
@@ -22,7 +22,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/optimizer/prep/prepunion.c,v 1.160 2008/10/22 20:17:52 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/optimizer/prep/prepunion.c,v 1.161 2008/11/11 18:13:32 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -91,11 +91,10 @@ static List *generate_append_tlist(List *colTypes, bool flag,
 static List *generate_setop_grouplist(SetOperationStmt *op, List *targetlist);
 static void expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte,
                                                 Index rti);
-static void make_inh_translation_lists(Relation oldrelation,
-                                                  Relation newrelation,
-                                                  Index newvarno,
-                                                  List **col_mappings,
-                                                  List **translated_vars);
+static void make_inh_translation_list(Relation oldrelation,
+                                                 Relation newrelation,
+                                                 Index newvarno,
+                                                 List **translated_vars);
 static Node *adjust_appendrel_attrs_mutator(Node *node,
                                                           AppendRelInfo *context);
 static Relids adjust_relid_set(Relids relids, Index oldrelid, Index newrelid);
@@ -1279,9 +1278,8 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
                appinfo->child_relid = childRTindex;
                appinfo->parent_reltype = oldrelation->rd_rel->reltype;
                appinfo->child_reltype = newrelation->rd_rel->reltype;
-               make_inh_translation_lists(oldrelation, newrelation, childRTindex,
-                                                                  &appinfo->col_mappings,
-                                                                  &appinfo->translated_vars);
+               make_inh_translation_list(oldrelation, newrelation, childRTindex,
+                                                                 &appinfo->translated_vars);
                appinfo->parent_reloid = parentOID;
                appinfos = lappend(appinfos, appinfo);
 
@@ -1316,19 +1314,17 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
 }
 
 /*
- * make_inh_translation_lists
- *       Build the lists of translations from parent Vars to child Vars for
- *       an inheritance child.  We need both a column number mapping list
- *       and a list of Vars representing the child columns.
+ * make_inh_translation_list
+ *       Build the list of translations from parent Vars to child Vars for
+ *       an inheritance child.
  *
  * For paranoia's sake, we match type as well as attribute name.
  */
 static void
-make_inh_translation_lists(Relation oldrelation, Relation newrelation,
-                                                  Index newvarno,
-                                                  List **col_mappings, List **translated_vars)
+make_inh_translation_list(Relation oldrelation, Relation newrelation,
+                                                 Index newvarno,
+                                                 List **translated_vars)
 {
-       List       *numbers = NIL;
        List       *vars = NIL;
        TupleDesc       old_tupdesc = RelationGetDescr(oldrelation);
        TupleDesc       new_tupdesc = RelationGetDescr(newrelation);
@@ -1347,8 +1343,7 @@ make_inh_translation_lists(Relation oldrelation, Relation newrelation,
                att = old_tupdesc->attrs[old_attno];
                if (att->attisdropped)
                {
-                       /* Just put 0/NULL into this list entry */
-                       numbers = lappend_int(numbers, 0);
+                       /* Just put NULL into this list entry */
                        vars = lappend(vars, NULL);
                        continue;
                }
@@ -1362,7 +1357,6 @@ make_inh_translation_lists(Relation oldrelation, Relation newrelation,
                 */
                if (oldrelation == newrelation)
                {
-                       numbers = lappend_int(numbers, old_attno + 1);
                        vars = lappend(vars, makeVar(newvarno,
                                                                                 (AttrNumber) (old_attno + 1),
                                                                                 atttypid,
@@ -1405,7 +1399,6 @@ make_inh_translation_lists(Relation oldrelation, Relation newrelation,
                        elog(ERROR, "attribute \"%s\" of relation \"%s\" does not match parent's type",
                                 attname, RelationGetRelationName(newrelation));
 
-               numbers = lappend_int(numbers, new_attno + 1);
                vars = lappend(vars, makeVar(newvarno,
                                                                         (AttrNumber) (new_attno + 1),
                                                                         atttypid,
@@ -1413,7 +1406,6 @@ make_inh_translation_lists(Relation oldrelation, Relation newrelation,
                                                                         0));
        }
 
-       *col_mappings = numbers;
        *translated_vars = vars;
 }
 
@@ -1682,70 +1674,6 @@ adjust_relid_set(Relids relids, Index oldrelid, Index newrelid)
        return relids;
 }
 
-/*
- * adjust_appendrel_attr_needed
- *             Adjust an attr_needed[] array to reference a member rel instead of
- *             the original appendrel
- *
- * oldrel: source of data (we use the attr_needed, min_attr, max_attr fields)
- * appinfo: supplies parent_relid, child_relid, col_mappings
- * new_min_attr, new_max_attr: desired bounds of new attr_needed array
- *
- * The relid sets are adjusted by substituting child_relid for parent_relid.
- * (NOTE: oldrel is not necessarily the parent_relid relation!)  We are also
- * careful to map attribute numbers within the array properly. User
- * attributes have to be mapped through col_mappings, but system attributes
- * and whole-row references always have the same attno.
- *
- * Returns a palloc'd array with the specified bounds
- */
-Relids *
-adjust_appendrel_attr_needed(RelOptInfo *oldrel, AppendRelInfo *appinfo,
-                                                        AttrNumber new_min_attr, AttrNumber new_max_attr)
-{
-       Relids     *new_attr_needed;
-       Index           parent_relid = appinfo->parent_relid;
-       Index           child_relid = appinfo->child_relid;
-       int                     parent_attr;
-       ListCell   *lm;
-
-       /* Create empty result array */
-       new_attr_needed = (Relids *)
-               palloc0((new_max_attr - new_min_attr + 1) * sizeof(Relids));
-       /* Process user attributes, with appropriate attno mapping */
-       parent_attr = 1;
-       foreach(lm, appinfo->col_mappings)
-       {
-               int                     child_attr = lfirst_int(lm);
-
-               if (child_attr > 0)
-               {
-                       Relids          attrneeded;
-
-                       Assert(parent_attr <= oldrel->max_attr);
-                       Assert(child_attr <= new_max_attr);
-                       attrneeded = oldrel->attr_needed[parent_attr - oldrel->min_attr];
-                       attrneeded = adjust_relid_set(attrneeded,
-                                                                                 parent_relid, child_relid);
-                       new_attr_needed[child_attr - new_min_attr] = attrneeded;
-               }
-               parent_attr++;
-       }
-       /* Process system attributes, including whole-row references */
-       Assert(new_min_attr <= oldrel->min_attr);
-       for (parent_attr = oldrel->min_attr; parent_attr <= 0; parent_attr++)
-       {
-               Relids          attrneeded;
-
-               attrneeded = oldrel->attr_needed[parent_attr - oldrel->min_attr];
-               attrneeded = adjust_relid_set(attrneeded,
-                                                                         parent_relid, child_relid);
-               new_attr_needed[parent_attr - new_min_attr] = attrneeded;
-       }
-
-       return new_attr_needed;
-}
-
 /*
  * Adjust the targetlist entries of an inherited UPDATE operation
  *
@@ -1778,24 +1706,24 @@ adjust_inherited_tlist(List *tlist, AppendRelInfo *context)
        foreach(tl, tlist)
        {
                TargetEntry *tle = (TargetEntry *) lfirst(tl);
-               int                     newattno;
+               Var                *childvar;
 
                if (tle->resjunk)
                        continue;                       /* ignore junk items */
 
-               /* Look up the translation of this column */
+               /* Look up the translation of this column: it must be a Var */
                if (tle->resno <= 0 ||
-                       tle->resno > list_length(context->col_mappings))
+                       tle->resno > list_length(context->translated_vars))
                        elog(ERROR, "attribute %d of relation \"%s\" does not exist",
                                 tle->resno, get_rel_name(context->parent_reloid));
-               newattno = list_nth_int(context->col_mappings, tle->resno - 1);
-               if (newattno <= 0)
+               childvar = (Var *) list_nth(context->translated_vars, tle->resno - 1);
+               if (childvar == NULL || !IsA(childvar, Var))
                        elog(ERROR, "attribute %d of relation \"%s\" does not exist",
                                 tle->resno, get_rel_name(context->parent_reloid));
 
-               if (tle->resno != newattno)
+               if (tle->resno != childvar->varattno)
                {
-                       tle->resno = newattno;
+                       tle->resno = childvar->varattno;
                        changed_it = true;
                }
        }
index 4ce13f808a29e108de8fa8a6ee7c0781643b47f2..56074a469dc2eb3392c87b99fb1b89983cbe4c54 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/relation.h,v 1.163 2008/10/22 20:17:52 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/relation.h,v 1.164 2008/11/11 18:13:32 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1239,27 +1239,14 @@ typedef struct AppendRelInfo
        Oid                     parent_reltype; /* OID of parent's composite type */
        Oid                     child_reltype;  /* OID of child's composite type */
 
-       /*
-        * The N'th element of this list is the integer column number of the child
-        * column corresponding to the N'th column of the parent. A list element
-        * is zero if it corresponds to a dropped column of the parent (this is
-        * only possible for inheritance cases, not UNION ALL).
-        */
-       List       *col_mappings;       /* list of child attribute numbers */
-
        /*
         * The N'th element of this list is a Var or expression representing the
         * child column corresponding to the N'th column of the parent. This is
         * used to translate Vars referencing the parent rel into references to
         * the child.  A list element is NULL if it corresponds to a dropped
         * column of the parent (this is only possible for inheritance cases, not
-        * UNION ALL).
-        *
-        * This might seem redundant with the col_mappings data, but it is handy
-        * because flattening of sub-SELECTs that are members of a UNION ALL will
-        * cause changes in the expressions that need to be substituted for a
-        * parent Var.  Adjusting this data structure lets us track what really
-        * needs to be substituted.
+        * UNION ALL).  The list elements are always simple Vars for inheritance
+        * cases, but can be arbitrary expressions in UNION ALL cases.
         *
         * Notice we only store entries for user columns (attno > 0).  Whole-row
         * Vars are special-cased, and system columns (attno < 0) need no special
index 9326623b91a4106062bebedfa0d0ee0966f6fc15..b6651866e7b7a8a695e45f6fcc7b313ededd4ae5 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/optimizer/prep.h,v 1.62 2008/08/17 01:20:00 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/optimizer/prep.h,v 1.63 2008/11/11 18:13:32 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -51,9 +51,4 @@ extern void expand_inherited_tables(PlannerInfo *root);
 
 extern Node *adjust_appendrel_attrs(Node *node, AppendRelInfo *appinfo);
 
-extern Relids *adjust_appendrel_attr_needed(RelOptInfo *oldrel,
-                                                        AppendRelInfo *appinfo,
-                                                        AttrNumber new_min_attr,
-                                                        AttrNumber new_max_attr);
-
 #endif   /* PREP_H */