(2014/07/01 11:10), Etsuro Fujita wrote:
> (2014/06/30 22:48), Tom Lane wrote:
>> Etsuro Fujita <fujita.ets...@lab.ntt.co.jp> writes:
>>> Done. I think this is because create_foreignscan_plan() makes reference
>>> to attr_needed, which isn't computed for inheritance children.
>>
>> I wonder whether it isn't time to change that. It was coded like that
>> originally only because calculating the values would've been a waste of
>> cycles at the time. But this is at least the third place where it'd be
>> useful to have attr_needed for child rels.
>
> +1 for calculating attr_needed for child rels. (I was wondering too.)
>
> I'll create a separate patch for it.
Attached is a WIP patch for that. The following functions have been
changed to refer to attr_needed:
* check_index_only()
* remove_unused_subquery_outputs()
* check_selective_binary_conversion()
I'll add this to the upcoming commitfest. If anyone has any time to
glance at it before then, that would be a great help.
Thanks,
Best regards,
Etsuro Fujita
*** a/contrib/file_fdw/file_fdw.c
--- b/contrib/file_fdw/file_fdw.c
***************
*** 799,806 **** check_selective_binary_conversion(RelOptInfo *baserel,
}
/* Collect all the attributes needed for joins or final output. */
! pull_varattnos((Node *) baserel->reltargetlist, baserel->relid,
! &attrs_used);
/* Add all the attributes used by restriction clauses. */
foreach(lc, baserel->baserestrictinfo)
--- 799,811 ----
}
/* Collect all the attributes needed for joins or final output. */
! for (i = baserel->min_attr; i <= baserel->max_attr; i++)
! {
! if (!bms_is_empty(baserel->attr_needed[i - baserel->min_attr]))
! attrs_used =
! bms_add_member(attrs_used,
! i - FirstLowInvalidHeapAttributeNumber);
! }
/* Add all the attributes used by restriction clauses. */
foreach(lc, baserel->baserestrictinfo)
*** a/src/backend/optimizer/path/allpaths.c
--- b/src/backend/optimizer/path/allpaths.c
***************
*** 577,588 **** set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
childrel->has_eclass_joins = rel->has_eclass_joins;
/*
! * 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.
*/
/*
* Compute the child's size.
--- 577,585 ----
childrel->has_eclass_joins = rel->has_eclass_joins;
/*
! * Compute the child's attr_needed.
*/
+ adjust_appendrel_attr_needed(rel, childrel, childRTE, appinfo);
/*
* Compute the child's size.
***************
*** 2173,2178 **** remove_unused_subquery_outputs(Query *subquery, RelOptInfo *rel)
--- 2170,2176 ----
{
Bitmapset *attrs_used = NULL;
ListCell *lc;
+ int i;
/*
* Do nothing if subquery has UNION/INTERSECT/EXCEPT: in principle we
***************
*** 2193,2204 **** remove_unused_subquery_outputs(Query *subquery, RelOptInfo *rel)
* Collect a bitmap of all the output column numbers used by the upper
* query.
*
! * Add all the attributes needed for joins or final output. Note: we must
! * look at reltargetlist, not the attr_needed data, because attr_needed
! * isn't computed for inheritance child rels, cf set_append_rel_size().
! * (XXX might be worth changing that sometime.)
*/
! pull_varattnos((Node *) rel->reltargetlist, rel->relid, &attrs_used);
/* Add all the attributes used by un-pushed-down restriction clauses. */
foreach(lc, rel->baserestrictinfo)
--- 2191,2205 ----
* Collect a bitmap of all the output column numbers used by the upper
* query.
*
! * Add all the attributes needed for joins or final output.
*/
! for (i = rel->min_attr; i <= rel->max_attr; i++)
! {
! if (!bms_is_empty(rel->attr_needed[i - rel->min_attr]))
! attrs_used =
! bms_add_member(attrs_used,
! i - FirstLowInvalidHeapAttributeNumber);
! }
/* Add all the attributes used by un-pushed-down restriction clauses. */
foreach(lc, rel->baserestrictinfo)
*** a/src/backend/optimizer/path/indxpath.c
--- b/src/backend/optimizer/path/indxpath.c
***************
*** 1768,1779 **** check_index_only(RelOptInfo *rel, IndexOptInfo *index)
* way out.
*/
! /*
! * Add all the attributes needed for joins or final output. Note: we must
! * look at reltargetlist, not the attr_needed data, because attr_needed
! * isn't computed for inheritance child rels.
! */
! pull_varattnos((Node *) rel->reltargetlist, rel->relid, &attrs_used);
/* Add all the attributes used by restriction clauses. */
foreach(lc, rel->baserestrictinfo)
--- 1768,1781 ----
* way out.
*/
! /* Collect all the attributes needed for joins or final output. */
! for (i = rel->min_attr; i <= rel->max_attr; i++)
! {
! if (!bms_is_empty(rel->attr_needed[i - rel->min_attr]))
! attrs_used =
! bms_add_member(attrs_used,
! i - FirstLowInvalidHeapAttributeNumber);
! }
/* Add all the attributes used by restriction clauses. */
foreach(lc, rel->baserestrictinfo)
*** a/src/backend/optimizer/prep/prepunion.c
--- b/src/backend/optimizer/prep/prepunion.c
***************
*** 109,114 **** static Bitmapset *translate_col_privs(const Bitmapset *parent_privs,
--- 109,122 ----
List *translated_vars);
static Node *adjust_appendrel_attrs_mutator(Node *node,
adjust_appendrel_attrs_context *context);
+ static void adjust_appendrel_attr_needed_inh(RelOptInfo *oldrel,
+ RelOptInfo *newrel,
+ RangeTblEntry *newrte,
+ AppendRelInfo *appinfo);
+ static void adjust_appendrel_attr_needed_setop(RelOptInfo *oldrel,
+ RelOptInfo *newrel,
+ RangeTblEntry *newrte,
+ AppendRelInfo *appinfo);
static Relids adjust_relid_set(Relids relids, Index oldrelid, Index newrelid);
static List *adjust_inherited_tlist(List *tlist,
AppendRelInfo *context);
***************
*** 1867,1872 **** adjust_appendrel_attrs_mutator(Node *node,
--- 1875,2002 ----
}
/*
+ * adjust_appendrel_attr_needed
+ * Compute the child's attr_needed by transforming the parent's attr_needed
+ * through the translated_vars mapping of the specified AppendRelInfo.
+ */
+ void
+ adjust_appendrel_attr_needed(RelOptInfo *oldrel,
+ RelOptInfo *newrel,
+ RangeTblEntry *newrte,
+ AppendRelInfo *appinfo)
+ {
+ if (OidIsValid(appinfo->parent_reloid))
+ adjust_appendrel_attr_needed_inh(oldrel, newrel, newrte, appinfo);
+ else
+ adjust_appendrel_attr_needed_setop(oldrel, newrel, newrte, appinfo);
+ }
+
+ static void
+ adjust_appendrel_attr_needed_inh(RelOptInfo *oldrel,
+ RelOptInfo *newrel,
+ RangeTblEntry *newrte,
+ AppendRelInfo *appinfo)
+ {
+ int i;
+ ListCell *lc;
+
+ Assert(newrte->rtekind == RTE_RELATION);
+
+ /*
+ * System attributes and whole-row Vars have the same numbers in all tables.
+ */
+ for (i = newrel->min_attr; i <= InvalidAttrNumber; i++)
+ newrel->attr_needed[i - newrel->min_attr] =
+ adjust_relid_set(oldrel->attr_needed[i - newrel->min_attr],
+ appinfo->parent_relid, appinfo->child_relid);
+
+ /*
+ * Compute user attributes' attr_needed through the translated_vars mapping.
+ */
+ i = InvalidAttrNumber;
+ foreach(lc, appinfo->translated_vars)
+ {
+ Var *var = (Var *) lfirst(lc);
+ int oldattoff;
+
+ i++;
+ if (var == NULL) /* ignore dropped columns */
+ continue;
+ Assert(IsA(var, Var));
+
+ if (!bms_is_empty(oldrel->attr_needed[i - newrel->min_attr]))
+ newrel->attr_needed[var->varattno - oldrel->min_attr] =
+ adjust_relid_set(oldrel->attr_needed[i - newrel->min_attr],
+ appinfo->parent_relid, appinfo->child_relid);
+ }
+ }
+
+ static void
+ adjust_appendrel_attr_needed_setop(RelOptInfo *oldrel,
+ RelOptInfo *newrel,
+ RangeTblEntry *newrte,
+ AppendRelInfo *appinfo)
+ {
+ int i;
+ ListCell *lc;
+ bool whole_row;
+
+ if (newrte->rtekind != RTE_RELATION)
+ {
+ for (i = oldrel->min_attr; i <= oldrel->max_attr; i++)
+ newrel->attr_needed[i] = adjust_relid_set(oldrel->attr_needed[i],
+ appinfo->parent_relid,
+ appinfo->child_relid);
+ return;
+ }
+
+ /*
+ * Compute the child's attr_needed by transforming the parent's attr_needed
+ * through the translated_vars mapping of the specified AppendRelInfo.
+ */
+
+ /* Check if parent has whole-row reference */
+ whole_row = !bms_is_empty(oldrel->attr_needed[InvalidAttrNumber]);
+
+ i = InvalidAttrNumber;
+ foreach(lc, appinfo->translated_vars)
+ {
+ Node *node = (Node *) lfirst(lc);
+ bool result;
+
+ i++;
+ if (whole_row || !bms_is_empty(oldrel->attr_needed[i]))
+ {
+ Bitmapset *attrs_used = NULL;
+ Bitmapset *tmpset = NULL;
+ int j;
+
+ pull_varattnos(node, newrel->relid, &attrs_used);
+ tmpset = bms_copy(attrs_used);
+ while ((j = bms_first_member(tmpset)) >= 0)
+ {
+ if (whole_row)
+ newrel->attr_needed[j - 1] =
+ bms_add_members(newrel->attr_needed[j - 1],
+ oldrel->attr_needed[InvalidAttrNumber]);
+ if (!bms_is_empty(oldrel->attr_needed[i]))
+ newrel->attr_needed[j - 1] =
+ bms_add_members(newrel->attr_needed[j - 1],
+ oldrel->attr_needed[i]);
+ }
+ bms_free(tmpset);
+ bms_free(attrs_used);
+ }
+ }
+
+ /* Adjust attr_needed's relid sets */
+ for (i = newrel->min_attr; i <= newrel->max_attr; i++)
+ newrel->attr_needed[i - newrel->min_attr] =
+ adjust_relid_set(newrel->attr_needed[i - newrel->min_attr],
+ appinfo->parent_relid, appinfo->child_relid);
+ }
+
+ /*
* Substitute newrelid for oldrelid in a Relid set
*/
static Relids
*** a/src/include/optimizer/prep.h
--- b/src/include/optimizer/prep.h
***************
*** 58,61 **** extern void expand_inherited_tables(PlannerInfo *root);
--- 58,66 ----
extern Node *adjust_appendrel_attrs(PlannerInfo *root, Node *node,
AppendRelInfo *appinfo);
+ extern void adjust_appendrel_attr_needed(RelOptInfo *oldrel,
+ RelOptInfo *newrel,
+ RangeTblEntry *newrte,
+ AppendRelInfo *appinfo);
+
#endif /* PREP_H */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
