Fix nested PlaceHolderVar expressions that appear only in targetlists.
A PlaceHolderVar's expression might contain another, lower-level PlaceHolderVar. If the outer PlaceHolderVar is used, the inner one certainly will be also, and so we have to make sure that both of them get into the placeholder_list with correct ph_may_need values during the initial pre-scan of the query (before deconstruct_jointree starts). We did this correctly for PlaceHolderVars appearing in the query quals, but overlooked the issue for those appearing in the top-level targetlist; with the result that nested placeholders referenced only in the targetlist did not work correctly, as illustrated in bug #6154. While at it, add some error checking to find_placeholder_info to ensure that we don't try to create new placeholders after it's too late to do so; they have to all be created before deconstruct_jointree starts. Back-patch to 8.4 where the PlaceHolderVar mechanism was introduced.
This commit is contained in:
Tom Lane
parent
d82a9d2a60
commit
77ba232564
|
|
@ -3491,7 +3491,7 @@ set_rel_width(PlannerInfo *root, RelOptInfo *rel)
|
|||
else if (IsA(node, PlaceHolderVar))
|
||||
{
|
||||
PlaceHolderVar *phv = (PlaceHolderVar *) node;
|
||||
PlaceHolderInfo *phinfo = find_placeholder_info(root, phv);
|
||||
PlaceHolderInfo *phinfo = find_placeholder_info(root, phv, false);
|
||||
|
||||
tuple_width += phinfo->ph_width;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -850,7 +850,7 @@ generate_base_implied_equalities_no_const(PlannerInfo *root,
|
|||
PVC_RECURSE_AGGREGATES,
|
||||
PVC_INCLUDE_PLACEHOLDERS);
|
||||
|
||||
add_vars_to_targetlist(root, vars, ec->ec_relids);
|
||||
add_vars_to_targetlist(root, vars, ec->ec_relids, false);
|
||||
list_free(vars);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -137,7 +137,7 @@ build_base_rel_tlists(PlannerInfo *root, List *final_tlist)
|
|||
|
||||
if (tlist_vars != NIL)
|
||||
{
|
||||
add_vars_to_targetlist(root, tlist_vars, bms_make_singleton(0));
|
||||
add_vars_to_targetlist(root, tlist_vars, bms_make_singleton(0), true);
|
||||
list_free(tlist_vars);
|
||||
}
|
||||
}
|
||||
|
|
@ -151,10 +151,15 @@ build_base_rel_tlists(PlannerInfo *root, List *final_tlist)
|
|||
*
|
||||
* The list may also contain PlaceHolderVars. These don't necessarily
|
||||
* have a single owning relation; we keep their attr_needed info in
|
||||
* root->placeholder_list instead.
|
||||
* root->placeholder_list instead. If create_new_ph is true, it's OK
|
||||
* to create new PlaceHolderInfos, and we also have to update ph_may_need;
|
||||
* otherwise, the PlaceHolderInfos must already exist, and we should only
|
||||
* update their ph_needed. (It should be true before deconstruct_jointree
|
||||
* begins, and false after that.)
|
||||
*/
|
||||
void
|
||||
add_vars_to_targetlist(PlannerInfo *root, List *vars, Relids where_needed)
|
||||
add_vars_to_targetlist(PlannerInfo *root, List *vars,
|
||||
Relids where_needed, bool create_new_ph)
|
||||
{
|
||||
ListCell *temp;
|
||||
|
||||
|
|
@ -185,18 +190,20 @@ add_vars_to_targetlist(PlannerInfo *root, List *vars, Relids where_needed)
|
|||
else if (IsA(node, PlaceHolderVar))
|
||||
{
|
||||
PlaceHolderVar *phv = (PlaceHolderVar *) node;
|
||||
PlaceHolderInfo *phinfo = find_placeholder_info(root, phv);
|
||||
PlaceHolderInfo *phinfo = find_placeholder_info(root, phv,
|
||||
create_new_ph);
|
||||
|
||||
/* Always adjust ph_needed */
|
||||
phinfo->ph_needed = bms_add_members(phinfo->ph_needed,
|
||||
where_needed);
|
||||
|
||||
/*
|
||||
* Update ph_may_need too. This is currently only necessary when
|
||||
* being called from build_base_rel_tlists, but we may as well do
|
||||
* it always.
|
||||
* If we are creating PlaceHolderInfos, mark them with the
|
||||
* correct maybe-needed locations. Otherwise, it's too late to
|
||||
* change that.
|
||||
*/
|
||||
phinfo->ph_may_need = bms_add_members(phinfo->ph_may_need,
|
||||
where_needed);
|
||||
if (create_new_ph)
|
||||
mark_placeholder_maybe_needed(root, phinfo, where_needed);
|
||||
}
|
||||
else
|
||||
elog(ERROR, "unrecognized node type: %d", (int) nodeTag(node));
|
||||
|
|
@ -1035,7 +1042,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
|
|||
PVC_RECURSE_AGGREGATES,
|
||||
PVC_INCLUDE_PLACEHOLDERS);
|
||||
|
||||
add_vars_to_targetlist(root, vars, relids);
|
||||
add_vars_to_targetlist(root, vars, relids, false);
|
||||
list_free(vars);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -24,7 +24,7 @@
|
|||
|
||||
/* Local functions */
|
||||
static Relids find_placeholders_recurse(PlannerInfo *root, Node *jtnode);
|
||||
static void find_placeholders_in_qual(PlannerInfo *root, Node *qual,
|
||||
static void mark_placeholders_in_expr(PlannerInfo *root, Node *expr,
|
||||
Relids relids);
|
||||
|
||||
|
||||
|
|
@ -50,7 +50,10 @@ make_placeholder_expr(PlannerInfo *root, Expr *expr, Relids phrels)
|
|||
|
||||
/*
|
||||
* find_placeholder_info
|
||||
* Fetch the PlaceHolderInfo for the given PHV; create it if not found
|
||||
* Fetch the PlaceHolderInfo for the given PHV
|
||||
*
|
||||
* If the PlaceHolderInfo doesn't exist yet, create it if create_new_ph is
|
||||
* true, else throw an error.
|
||||
*
|
||||
* This is separate from make_placeholder_expr because subquery pullup has
|
||||
* to make PlaceHolderVars for expressions that might not be used at all in
|
||||
|
|
@ -58,10 +61,13 @@ make_placeholder_expr(PlannerInfo *root, Expr *expr, Relids phrels)
|
|||
* We build PlaceHolderInfos only for PHVs that are still present in the
|
||||
* simplified query passed to query_planner().
|
||||
*
|
||||
* Note: this should only be called after query_planner() has started.
|
||||
* Note: this should only be called after query_planner() has started. Also,
|
||||
* create_new_ph must not be TRUE after deconstruct_jointree begins, because
|
||||
* make_outerjoininfo assumes that we already know about all placeholders.
|
||||
*/
|
||||
PlaceHolderInfo *
|
||||
find_placeholder_info(PlannerInfo *root, PlaceHolderVar *phv)
|
||||
find_placeholder_info(PlannerInfo *root, PlaceHolderVar *phv,
|
||||
bool create_new_ph)
|
||||
{
|
||||
PlaceHolderInfo *phinfo;
|
||||
ListCell *lc;
|
||||
|
|
@ -77,6 +83,9 @@ find_placeholder_info(PlannerInfo *root, PlaceHolderVar *phv)
|
|||
}
|
||||
|
||||
/* Not found, so create it */
|
||||
if (!create_new_ph)
|
||||
elog(ERROR, "too late to create a new PlaceHolderInfo");
|
||||
|
||||
phinfo = makeNode(PlaceHolderInfo);
|
||||
|
||||
phinfo->phid = phv->phid;
|
||||
|
|
@ -157,7 +166,7 @@ find_placeholders_recurse(PlannerInfo *root, Node *jtnode)
|
|||
/*
|
||||
* Now process the top-level quals.
|
||||
*/
|
||||
find_placeholders_in_qual(root, f->quals, jtrelids);
|
||||
mark_placeholders_in_expr(root, f->quals, jtrelids);
|
||||
}
|
||||
else if (IsA(jtnode, JoinExpr))
|
||||
{
|
||||
|
|
@ -173,7 +182,7 @@ find_placeholders_recurse(PlannerInfo *root, Node *jtnode)
|
|||
jtrelids = bms_join(leftids, rightids);
|
||||
|
||||
/* Process the qual clauses */
|
||||
find_placeholders_in_qual(root, j->quals, jtrelids);
|
||||
mark_placeholders_in_expr(root, j->quals, jtrelids);
|
||||
}
|
||||
else
|
||||
{
|
||||
|
|
@ -185,14 +194,15 @@ find_placeholders_recurse(PlannerInfo *root, Node *jtnode)
|
|||
}
|
||||
|
||||
/*
|
||||
* find_placeholders_in_qual
|
||||
* Process a qual clause for find_placeholders_in_jointree.
|
||||
* mark_placeholders_in_expr
|
||||
* Find all PlaceHolderVars in the given expression, and mark them
|
||||
* as possibly needed at the specified join level.
|
||||
*
|
||||
* relids is the syntactic join level to mark as the "maybe needed" level
|
||||
* for each PlaceHolderVar found in the qual clause.
|
||||
* for each PlaceHolderVar found in the expression.
|
||||
*/
|
||||
static void
|
||||
find_placeholders_in_qual(PlannerInfo *root, Node *qual, Relids relids)
|
||||
mark_placeholders_in_expr(PlannerInfo *root, Node *expr, Relids relids)
|
||||
{
|
||||
List *vars;
|
||||
ListCell *vl;
|
||||
|
|
@ -201,7 +211,7 @@ find_placeholders_in_qual(PlannerInfo *root, Node *qual, Relids relids)
|
|||
* pull_var_clause does more than we need here, but it'll do and it's
|
||||
* convenient to use.
|
||||
*/
|
||||
vars = pull_var_clause(qual,
|
||||
vars = pull_var_clause(expr,
|
||||
PVC_RECURSE_AGGREGATES,
|
||||
PVC_INCLUDE_PLACEHOLDERS);
|
||||
foreach(vl, vars)
|
||||
|
|
@ -214,25 +224,47 @@ find_placeholders_in_qual(PlannerInfo *root, Node *qual, Relids relids)
|
|||
continue;
|
||||
|
||||
/* Create a PlaceHolderInfo entry if there's not one already */
|
||||
phinfo = find_placeholder_info(root, phv);
|
||||
phinfo = find_placeholder_info(root, phv, true);
|
||||
|
||||
/* Mark the PHV as possibly needed at the qual's syntactic level */
|
||||
phinfo->ph_may_need = bms_add_members(phinfo->ph_may_need, relids);
|
||||
|
||||
/*
|
||||
* This is a bit tricky: the PHV's contained expression may contain
|
||||
* other, lower-level PHVs. We need to get those into the
|
||||
* PlaceHolderInfo list, but they aren't going to be needed where the
|
||||
* outer PHV is referenced. Rather, they'll be needed where the outer
|
||||
* PHV is evaluated. We can estimate that (conservatively) as the
|
||||
* syntactic location of the PHV's expression. Recurse to take care
|
||||
* of any such PHVs.
|
||||
*/
|
||||
find_placeholders_in_qual(root, (Node *) phv->phexpr, phv->phrels);
|
||||
/* Mark it, and recursively process any contained placeholders */
|
||||
mark_placeholder_maybe_needed(root, phinfo, relids);
|
||||
}
|
||||
list_free(vars);
|
||||
}
|
||||
|
||||
/*
|
||||
* mark_placeholder_maybe_needed
|
||||
* Mark a placeholder as possibly needed at the specified join level.
|
||||
*
|
||||
* relids is the syntactic join level to mark as the "maybe needed" level
|
||||
* for the placeholder.
|
||||
*
|
||||
* This is called during an initial scan of the query's targetlist and quals
|
||||
* before we begin deconstruct_jointree. Once we begin deconstruct_jointree,
|
||||
* all active placeholders must be present in root->placeholder_list with
|
||||
* their correct ph_may_need values, because make_outerjoininfo and
|
||||
* update_placeholder_eval_levels require this info to be available while
|
||||
* we crawl up the join tree.
|
||||
*/
|
||||
void
|
||||
mark_placeholder_maybe_needed(PlannerInfo *root, PlaceHolderInfo *phinfo,
|
||||
Relids relids)
|
||||
{
|
||||
/* Mark the PHV as possibly needed at the given syntactic level */
|
||||
phinfo->ph_may_need = bms_add_members(phinfo->ph_may_need, relids);
|
||||
|
||||
/*
|
||||
* This is a bit tricky: the PHV's contained expression may contain other,
|
||||
* lower-level PHVs. We need to get those into the PlaceHolderInfo list,
|
||||
* but they aren't going to be needed where the outer PHV is referenced.
|
||||
* Rather, they'll be needed where the outer PHV is evaluated. We can
|
||||
* estimate that (conservatively) as the syntactic location of the PHV's
|
||||
* expression. Recurse to take care of any such PHVs.
|
||||
*/
|
||||
mark_placeholders_in_expr(root, (Node *) phinfo->ph_var->phexpr,
|
||||
phinfo->ph_var->phrels);
|
||||
}
|
||||
|
||||
/*
|
||||
* update_placeholder_eval_levels
|
||||
* Adjust the target evaluation levels for placeholders
|
||||
|
|
@ -353,7 +385,7 @@ fix_placeholder_input_needed_levels(PlannerInfo *root)
|
|||
PVC_RECURSE_AGGREGATES,
|
||||
PVC_INCLUDE_PLACEHOLDERS);
|
||||
|
||||
add_vars_to_targetlist(root, vars, eval_at);
|
||||
add_vars_to_targetlist(root, vars, eval_at, false);
|
||||
list_free(vars);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -20,8 +20,10 @@
|
|||
extern PlaceHolderVar *make_placeholder_expr(PlannerInfo *root, Expr *expr,
|
||||
Relids phrels);
|
||||
extern PlaceHolderInfo *find_placeholder_info(PlannerInfo *root,
|
||||
PlaceHolderVar *phv);
|
||||
PlaceHolderVar *phv, bool create_new_ph);
|
||||
extern void find_placeholders_in_jointree(PlannerInfo *root);
|
||||
extern void mark_placeholder_maybe_needed(PlannerInfo *root,
|
||||
PlaceHolderInfo *phinfo, Relids relids);
|
||||
extern void update_placeholder_eval_levels(PlannerInfo *root,
|
||||
SpecialJoinInfo *new_sjinfo);
|
||||
extern void fix_placeholder_input_needed_levels(PlannerInfo *root);
|
||||
|
|
|
|||
|
|
@ -92,7 +92,7 @@ extern int join_collapse_limit;
|
|||
extern void add_base_rels_to_query(PlannerInfo *root, Node *jtnode);
|
||||
extern void build_base_rel_tlists(PlannerInfo *root, List *final_tlist);
|
||||
extern void add_vars_to_targetlist(PlannerInfo *root, List *vars,
|
||||
Relids where_needed);
|
||||
Relids where_needed, bool create_new_ph);
|
||||
extern List *deconstruct_jointree(PlannerInfo *root);
|
||||
extern void distribute_restrictinfo_to_rels(PlannerInfo *root,
|
||||
RestrictInfo *restrictinfo);
|
||||
|
|
|
|||
|
|
@ -2488,6 +2488,51 @@ order by c.name;
|
|||
(3 rows)
|
||||
|
||||
rollback;
|
||||
--
|
||||
-- test incorrect handling of placeholders that only appear in targetlists,
|
||||
-- per bug #6154
|
||||
--
|
||||
SELECT * FROM
|
||||
( SELECT 1 as key1 ) sub1
|
||||
LEFT JOIN
|
||||
( SELECT sub3.key3, sub4.value2, COALESCE(sub4.value2, 66) as value3 FROM
|
||||
( SELECT 1 as key3 ) sub3
|
||||
LEFT JOIN
|
||||
( SELECT sub5.key5, COALESCE(sub6.value1, 1) as value2 FROM
|
||||
( SELECT 1 as key5 ) sub5
|
||||
LEFT JOIN
|
||||
( SELECT 2 as key6, 42 as value1 ) sub6
|
||||
ON sub5.key5 = sub6.key6
|
||||
) sub4
|
||||
ON sub4.key5 = sub3.key3
|
||||
) sub2
|
||||
ON sub1.key1 = sub2.key3;
|
||||
key1 | key3 | value2 | value3
|
||||
------+------+--------+--------
|
||||
1 | 1 | 1 | 1
|
||||
(1 row)
|
||||
|
||||
-- test the path using join aliases, too
|
||||
SELECT * FROM
|
||||
( SELECT 1 as key1 ) sub1
|
||||
LEFT JOIN
|
||||
( SELECT sub3.key3, value2, COALESCE(value2, 66) as value3 FROM
|
||||
( SELECT 1 as key3 ) sub3
|
||||
LEFT JOIN
|
||||
( SELECT sub5.key5, COALESCE(sub6.value1, 1) as value2 FROM
|
||||
( SELECT 1 as key5 ) sub5
|
||||
LEFT JOIN
|
||||
( SELECT 2 as key6, 42 as value1 ) sub6
|
||||
ON sub5.key5 = sub6.key6
|
||||
) sub4
|
||||
ON sub4.key5 = sub3.key3
|
||||
) sub2
|
||||
ON sub1.key1 = sub2.key3;
|
||||
key1 | key3 | value2 | value3
|
||||
------+------+--------+--------
|
||||
1 | 1 | 1 | 1
|
||||
(1 row)
|
||||
|
||||
--
|
||||
-- test the corner cases FULL JOIN ON TRUE and FULL JOIN ON FALSE
|
||||
--
|
||||
|
|
|
|||
|
|
@ -602,6 +602,43 @@ order by c.name;
|
|||
|
||||
rollback;
|
||||
|
||||
--
|
||||
-- test incorrect handling of placeholders that only appear in targetlists,
|
||||
-- per bug #6154
|
||||
--
|
||||
SELECT * FROM
|
||||
( SELECT 1 as key1 ) sub1
|
||||
LEFT JOIN
|
||||
( SELECT sub3.key3, sub4.value2, COALESCE(sub4.value2, 66) as value3 FROM
|
||||
( SELECT 1 as key3 ) sub3
|
||||
LEFT JOIN
|
||||
( SELECT sub5.key5, COALESCE(sub6.value1, 1) as value2 FROM
|
||||
( SELECT 1 as key5 ) sub5
|
||||
LEFT JOIN
|
||||
( SELECT 2 as key6, 42 as value1 ) sub6
|
||||
ON sub5.key5 = sub6.key6
|
||||
) sub4
|
||||
ON sub4.key5 = sub3.key3
|
||||
) sub2
|
||||
ON sub1.key1 = sub2.key3;
|
||||
|
||||
-- test the path using join aliases, too
|
||||
SELECT * FROM
|
||||
( SELECT 1 as key1 ) sub1
|
||||
LEFT JOIN
|
||||
( SELECT sub3.key3, value2, COALESCE(value2, 66) as value3 FROM
|
||||
( SELECT 1 as key3 ) sub3
|
||||
LEFT JOIN
|
||||
( SELECT sub5.key5, COALESCE(sub6.value1, 1) as value2 FROM
|
||||
( SELECT 1 as key5 ) sub5
|
||||
LEFT JOIN
|
||||
( SELECT 2 as key6, 42 as value1 ) sub6
|
||||
ON sub5.key5 = sub6.key6
|
||||
) sub4
|
||||
ON sub4.key5 = sub3.key3
|
||||
) sub2
|
||||
ON sub1.key1 = sub2.key3;
|
||||
|
||||
--
|
||||
-- test the corner cases FULL JOIN ON TRUE and FULL JOIN ON FALSE
|
||||
--
|
||||
|
|
|
|||
Loading…
Reference in New Issue