database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-06-29 10:41:53 +03:00
Code Activity

Disable support for partitionwise joins in problematic cases.

Browse Source 
Commit f49842d, which added support for partitionwise joins, built the
child's tlist by applying adjust_appendrel_attrs() to the parent's.  So in
the case where the parent's included a whole-row Var for the parent, the
child's contained a ConvertRowtypeExpr.  To cope with that, that commit
added code to the planner, such as setrefs.c, but some code paths still
assumed that the tlist for a scan (or join) rel would only include Vars
and PlaceHolderVars, which was true before that commit, causing errors:

* When creating an explicit sort node for an input path for a mergejoin
  path for a child join, prepare_sort_from_pathkeys() threw the 'could not
  find pathkey item to sort' error.
* When deparsing a relation participating in a pushed down child join as a
  subquery in contrib/postgres_fdw, get_relation_column_alias_ids() threw
  the 'unexpected expression in subquery output' error.
* When performing set_plan_references() on a local join plan generated by
  contrib/postgres_fdw for EvalPlanQual support for a pushed down child
  join, fix_join_expr() threw the 'variable not found in subplan target
  lists' error.

To fix these, two approaches have been proposed: one by Ashutosh Bapat and
one by me.  While the former keeps building the child's tlist with a
ConvertRowtypeExpr, the latter builds it with a whole-row Var for the
child not to violate the planner assumption, and tries to fix it up later,
But both approaches need more work, so refuse to generate partitionwise
join paths when whole-row Vars are involved, instead.  We don't need to
handle ConvertRowtypeExprs in the child's tlists for now, so this commit
also removes the changes to the planner.

Previously, partitionwise join computed attr_needed data for each child
separately, and built the child join's tlist using that data, which also
required an extra step for adding PlaceHolderVars to that tlist, but it
would be more efficient to build it from the parent join's tlist through
the adjust_appendrel_attrs() transformation.  So this commit builds that
list that way, and simplifies build_joinrel_tlist() and placeholder.c as
well as part of set_append_rel_size() to basically what they were before
partitionwise join went in.

Back-patch to PG11 where partitionwise join was introduced.

Report by Rajkumar Raghuwanshi.  Analysis by Ashutosh Bapat, who also
provided some of regression tests.  Patch by me, reviewed by Robert Haas.

Discussion: https://postgr.es/m/CAKcux6ktu-8tefLWtQuuZBYFaZA83vUzuRd7c1YHC-yEWyYFpg@mail.gmail.com
This commit is contained in:
Etsuro Fujita
2018-08-31 20:34:06 +09:00
parent bb60f2cb6e
commit 7cfdc77023
13 changed files with 266 additions and 256 deletions
Download Patch File Download Diff File Expand all files Collapse all files

58
src/backend/optimizer/plan/setrefs.c
View File

@ -41,9 +41,6 @@ typedef struct
int num_vars; /* number of plain Var tlist entries */
bool has_ph_vars; /* are there PlaceHolderVar entries? */
bool has_non_vars; /* are there other entries? */
bool has_conv_whole_rows; /* are there ConvertRowtypeExpr
* entries encapsulating a whole-row
* Var? */
tlist_vinfo vars[FLEXIBLE_ARRAY_MEMBER]; /* has num_vars entries */
} indexed_tlist;
@ -143,7 +140,6 @@ static List *set_returning_clause_references(PlannerInfo *root,
int rtoffset);
static bool extract_query_dependencies_walker(Node *node,
PlannerInfo *context);
static bool is_converted_whole_row_reference(Node *node);
/*****************************************************************************
*
@ -1997,7 +1993,6 @@ build_tlist_index(List *tlist)
itlist->tlist = tlist;
itlist->has_ph_vars = false;
itlist->has_non_vars = false;
itlist->has_conv_whole_rows = false;
/* Find the Vars and fill in the index array */
vinfo = itlist->vars;
@ -2016,8 +2011,6 @@ build_tlist_index(List *tlist)
}
else if (tle->expr && IsA(tle->expr, PlaceHolderVar))
itlist->has_ph_vars = true;
else if (is_converted_whole_row_reference((Node *) tle->expr))
itlist->has_conv_whole_rows = true;
else
itlist->has_non_vars = true;
}
@ -2033,10 +2026,7 @@ build_tlist_index(List *tlist)
* This is like build_tlist_index, but we only index tlist entries that
* are Vars belonging to some rel other than the one specified. We will set
* has_ph_vars (allowing PlaceHolderVars to be matched), but not has_non_vars
* (so nothing other than Vars and PlaceHolderVars can be matched). In case of
* DML, where this function will be used, returning lists from child relations
* will be appended similar to a simple append relation. That does not require
* fixing ConvertRowtypeExpr references. So, those are not considered here.
* (so nothing other than Vars and PlaceHolderVars can be matched).
*/
static indexed_tlist *
build_tlist_index_other_vars(List *tlist, Index ignore_rel)
@ -2053,7 +2043,6 @@ build_tlist_index_other_vars(List *tlist, Index ignore_rel)
itlist->tlist = tlist;
itlist->has_ph_vars = false;
itlist->has_non_vars = false;
itlist->has_conv_whole_rows = false;
/* Find the desired Vars and fill in the index array */
vinfo = itlist->vars;
@ -2256,7 +2245,6 @@ static Node *
fix_join_expr_mutator(Node *node, fix_join_expr_context *context)
{
Var *newvar;
bool converted_whole_row;
if (node == NULL)
return NULL;
@ -2326,12 +2314,8 @@ fix_join_expr_mutator(Node *node, fix_join_expr_context *context)
}
if (IsA(node, Param))
return fix_param_node(context->root, (Param *) node);
/* Try matching more complex expressions too, if tlists have any */
converted_whole_row = is_converted_whole_row_reference(node);
if (context->outer_itlist &&
(context->outer_itlist->has_non_vars ||
(context->outer_itlist->has_conv_whole_rows && converted_whole_row)))
if (context->outer_itlist && context->outer_itlist->has_non_vars)
{
newvar = search_indexed_tlist_for_non_var((Expr *) node,
context->outer_itlist,
@ -2339,9 +2323,7 @@ fix_join_expr_mutator(Node *node, fix_join_expr_context *context)
if (newvar)
return (Node *) newvar;
}
if (context->inner_itlist &&
(context->inner_itlist->has_non_vars ||
(context->inner_itlist->has_conv_whole_rows && converted_whole_row)))
if (context->inner_itlist && context->inner_itlist->has_non_vars)
{
newvar = search_indexed_tlist_for_non_var((Expr *) node,
context->inner_itlist,
@ -2461,9 +2443,7 @@ fix_upper_expr_mutator(Node *node, fix_upper_expr_context *context)
/* If no match, just fall through to process it normally */
}
/* Try matching more complex expressions too, if tlist has any */
if (context->subplan_itlist->has_non_vars ||
(context->subplan_itlist->has_conv_whole_rows &&
is_converted_whole_row_reference(node)))
if (context->subplan_itlist->has_non_vars)
{
newvar = search_indexed_tlist_for_non_var((Expr *) node,
context->subplan_itlist,
@ -2670,33 +2650,3 @@ extract_query_dependencies_walker(Node *node, PlannerInfo *context)
return expression_tree_walker(node, extract_query_dependencies_walker,
(void *) context);
}
/*
* is_converted_whole_row_reference
* If the given node is a ConvertRowtypeExpr encapsulating a whole-row
* reference as implicit cast, return true. Otherwise return false.
*/
static bool
is_converted_whole_row_reference(Node *node)
{
ConvertRowtypeExpr *convexpr;
if (!node || !IsA(node, ConvertRowtypeExpr))
return false;
/* Traverse nested ConvertRowtypeExpr's. */
convexpr = castNode(ConvertRowtypeExpr, node);
while (convexpr->convertformat == COERCE_IMPLICIT_CAST &&
IsA(convexpr->arg, ConvertRowtypeExpr))
convexpr = castNode(ConvertRowtypeExpr, convexpr->arg);
if (IsA(convexpr->arg, Var))
{
Var *var = castNode(Var, convexpr->arg);
if (var->varattno == 0)
return true;
}
return false;
}