open-dsi
/
postgres
mirror of https://github.com/postgres/postgres
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Get rid of adjust_appendrel_attr_needed(), which has been broken ever since

Browse Source 
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.
REL8_5_ALPHA1_BRANCH
Tom Lane 17 years ago
parent ccc9073f26
commit 0436679969
8 changed files with 46 additions and 146 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 3
      src/backend/nodes/copyfuncs.c
  2. 3
      src/backend/nodes/equalfuncs.c
  3. 3
      src/backend/nodes/outfuncs.c
  4. 14
      src/backend/optimizer/path/allpaths.c
  5. 29
      src/backend/optimizer/prep/prepjointree.c
  6. 114
      src/backend/optimizer/prep/prepunion.c
  7. 19
      src/include/nodes/relation.h
  8. 7
      src/include/optimizer/prep.h

3
src/backend/nodes/copyfuncs.c
Unescape View 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);

3
src/backend/nodes/equalfuncs.c
Unescape View 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);

3
src/backend/nodes/outfuncs.c
Unescape View 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);
}

14
src/backend/optimizer/path/allpaths.c
Unescape View 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

29
src/backend/optimizer/prep/prepjointree.c
Unescape View 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;
}

114
src/backend/optimizer/prep/prepunion.c
Unescape View 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;
}
}

19
src/include/nodes/relation.h
Unescape View 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

7
src/include/optimizer/prep.h
Unescape View 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 */

Write Preview
Loading…
Cancel
Save