database/postgres
database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-05-21 15:54:08 +03:00
Code

Fix mishandling of tSRFs at different nesting levels.

Browse Source 
Given a targetlist like "srf(x), f(srf(x))", split_pathtarget_at_srfs()
decided that it needed two levels of ProjectSet nodes, failing to notice
that the two SRF calls are textually equal().  Because of that, setrefs.c
would convert the upper ProjectSet's tlist to "Var1, f(Var1)" (where Var1
represents a reference to the srf(x) output of the lower ProjectSet).
This triggered an assertion in nodeProjectSet.c complaining that it found
no SRFs to evaluate, as reported by Erik Rijkers.

What we want in such a case is to evaluate srf(x) only once and use a plain
Result node to compute "Var1, f(Var1)"; that gives results similar to what
previous versions produced, whereas allowing srf(x) to be evaluated again
in an upper ProjectSet would square the number of rows emitted.

Furthermore, even if the SRF calls aren't textually identical, we want them
to be evaluated in lockstep, because that's what happened in the old
implementation.  But split_pathtarget_at_srfs() got this completely wrong,
using two levels of ProjectSet for a case like "srf(x), f(srf(y))".

Hence, rewrite split_pathtarget_at_srfs() from the ground up so that it
groups SRFs according to the depth of nesting of SRFs in their arguments.
This is pretty much how we envisioned that working originally, but I blew
it when it came to implementation.

In passing, optimize the case of target == input_target, which I noticed
is not only possible but quite common.

Discussion: https://postgr.es/m/dcbd2853c05d22088766553d60dc78c6@xs4all.nl
This commit is contained in:
Tom Lane 2017-02-02 16:38:13 -05:00
parent ecb814b5ce
commit c82d4e658e
4 changed files with 372 additions and 104 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

352
src/backend/optimizer/util/tlist.c
View File

@ -20,10 +20,21 @@
#include "optimizer/tlist.h" #include "optimizer/tlist.h"
/* Test if an expression node represents a SRF call. Beware multiple eval! */
#define IS_SRF_CALL(node) \
((IsA(node, FuncExpr) && ((FuncExpr *) (node))->funcretset) || \
(IsA(node, OpExpr) && ((OpExpr *) (node))->opretset))
/* Workspace for split_pathtarget_walker */
typedef struct typedef struct
{ {
List *nextlevel_tlist; List *input_target_exprs; /* exprs available from input */
bool nextlevel_contains_srfs; List *level_srfs; /* list of lists of SRF exprs */
List *level_input_vars; /* vars needed by SRFs of each level */
List *level_input_srfs; /* SRFs needed by SRFs of each level */
List *current_input_vars; /* vars needed in current subexpr */
List *current_input_srfs; /* SRFs needed in current subexpr */
int current_depth; /* max SRF depth in current subexpr */
} split_pathtarget_context; } split_pathtarget_context;
static bool split_pathtarget_walker(Node *node, static bool split_pathtarget_walker(Node *node,
@ -799,24 +810,27 @@ apply_pathtarget_labeling_to_tlist(List *tlist, PathTarget *target)
* x, y, z * x, y, z
* which satisfy the desired property. * which satisfy the desired property.
* *
* Another example is
* srf1(x), srf2(srf3(y))
* That must appear as-is in the top PathTarget, but below that we need
* srf1(x), srf3(y)
* That is, each SRF must be computed at a level corresponding to the nesting
* depth of SRFs within its arguments.
*
* In some cases, a SRF has already been evaluated in some previous plan level * In some cases, a SRF has already been evaluated in some previous plan level
* and we shouldn't expand it again (that is, what we see in the target is * and we shouldn't expand it again (that is, what we see in the target is
* already meant as a reference to a lower subexpression). So, don't expand * already meant as a reference to a lower subexpression). So, don't expand
* any tlist expressions that appear in input_target, if that's not NULL. * any tlist expressions that appear in input_target, if that's not NULL.
* In principle we might need to consider matching subexpressions to
* input_target, but for now it's not necessary because only ORDER BY and
* GROUP BY expressions are at issue and those will look the same at both
* plan levels.
* *
* The outputs of this function are two parallel lists, one a list of * The outputs of this function are two parallel lists, one a list of
* PathTargets and the other an integer list of bool flags indicating * PathTargets and the other an integer list of bool flags indicating
* whether the corresponding PathTarget contains any top-level SRFs. * whether the corresponding PathTarget contains any evaluatable SRFs.
* The lists are given in the order they'd need to be evaluated in, with * The lists are given in the order they'd need to be evaluated in, with
* the "lowest" PathTarget first. So the last list entry is always the * the "lowest" PathTarget first. So the last list entry is always the
* originally given PathTarget, and any entries before it indicate evaluation * originally given PathTarget, and any entries before it indicate evaluation
* levels that must be inserted below it. The first list entry must not * levels that must be inserted below it. The first list entry must not
* contain any SRFs, since it will typically be attached to a plan node * contain any SRFs (other than ones duplicating input_target entries), since
* that cannot evaluate SRFs. * it will typically be attached to a plan node that cannot evaluate SRFs.
* *
* Note: using a list for the flags may seem like overkill, since there * Note: using a list for the flags may seem like overkill, since there
* are only a few possible patterns for which levels contain SRFs. * are only a few possible patterns for which levels contain SRFs.
@ -827,133 +841,283 @@ split_pathtarget_at_srfs(PlannerInfo *root,
PathTarget *target, PathTarget *input_target, PathTarget *target, PathTarget *input_target,
List **targets, List **targets_contain_srfs) List **targets, List **targets_contain_srfs)
{ {
/* Initialize output lists to empty; we prepend to them within loop */
*targets = *targets_contain_srfs = NIL;
/* Loop to consider each level of PathTarget we need */
for (;;)
{
bool target_contains_srfs = false;
split_pathtarget_context context; split_pathtarget_context context;
ListCell *lc; int max_depth;
bool need_extra_projection;
context.nextlevel_tlist = NIL; List *prev_level_tlist;
context.nextlevel_contains_srfs = false; ListCell *lc,
*lc1,
*lc2,
*lc3;
/* /*
* Scan the PathTarget looking for SRFs. Top-level SRFs are handled * It's not unusual for planner.c to pass us two physically identical
* in this loop, ones lower down are found by split_pathtarget_walker. * targets, in which case we can conclude without further ado that all
* expressions are available from the input. (The logic below would
* arrive at the same conclusion, but much more tediously.)
*/ */
if (target == input_target)
{
*targets = list_make1(target);
*targets_contain_srfs = list_make1_int(false);
return;
}
/* Pass any input_target exprs down to split_pathtarget_walker() */
context.input_target_exprs = input_target ? input_target->exprs : NIL;
/*
* Initialize with empty level-zero lists, and no levels after that.
* (Note: we could dispense with representing level zero explicitly, since
* it will never receive any SRFs, but then we'd have to special-case that
* level when we get to building result PathTargets. Level zero describes
* the SRF-free PathTarget that will be given to the input plan node.)
*/
context.level_srfs = list_make1(NIL);
context.level_input_vars = list_make1(NIL);
context.level_input_srfs = list_make1(NIL);
/* Initialize data we'll accumulate across all the target expressions */
context.current_input_vars = NIL;
context.current_input_srfs = NIL;
max_depth = 0;
need_extra_projection = false;
/* Scan each expression in the PathTarget looking for SRFs */
foreach(lc, target->exprs) foreach(lc, target->exprs)
{ {
Node *node = (Node *) lfirst(lc); Node *node = (Node *) lfirst(lc);
/* /*
* A tlist item that is just a reference to an expression already * Find all SRFs and Vars (and Var-like nodes) in this expression, and
* computed in input_target need not be evaluated here, so just * enter them into appropriate lists within the context struct.
* make sure it's included in the next PathTarget.
*/ */
if (input_target && list_member(input_target->exprs, node)) context.current_depth = 0;
{ split_pathtarget_walker(node, &context);
context.nextlevel_tlist = lappend(context.nextlevel_tlist, node);
/* An expression containing no SRFs is of no further interest */
if (context.current_depth == 0)
continue; continue;
/*
* Track max SRF nesting depth over the whole PathTarget. Also, if
* this expression establishes a new max depth, we no longer care
* whether previous expressions contained nested SRFs; we can handle
* any required projection for them in the final ProjectSet node.
*/
if (max_depth < context.current_depth)
{
max_depth = context.current_depth;
need_extra_projection = false;
} }
/* Else, we need to compute this expression. */ /*
if (IsA(node, FuncExpr) && * If any maximum-depth SRF is not at the top level of its expression,
((FuncExpr *) node)->funcretset) * we'll need an extra Result node to compute the top-level scalar
{ * expression.
/* Top-level SRF: it can be evaluated here */ */
target_contains_srfs = true; if (max_depth == context.current_depth && !IS_SRF_CALL(node))
/* Recursively examine SRF's inputs */ need_extra_projection = true;
split_pathtarget_walker((Node *) ((FuncExpr *) node)->args,
&context);
} }
else if (IsA(node, OpExpr) &&
((OpExpr *) node)->opretset) /*
* If we found no SRFs needing evaluation (maybe they were all present in
* input_target, or maybe they were all removed by const-simplification),
* then no ProjectSet is needed; fall out.
*/
if (max_depth == 0)
{ {
/* Same as above, but for set-returning operator */ *targets = list_make1(target);
target_contains_srfs = true; *targets_contain_srfs = list_make1_int(false);
split_pathtarget_walker((Node *) ((OpExpr *) node)->args, return;
&context); }
/*
* The Vars and SRF outputs needed at top level can be added to the last
* level_input lists if we don't need an extra projection step. If we do
* need one, add a SRF-free level to the lists.
*/
if (need_extra_projection)
{
context.level_srfs = lappend(context.level_srfs, NIL);
context.level_input_vars = lappend(context.level_input_vars,
context.current_input_vars);
context.level_input_srfs = lappend(context.level_input_srfs,
context.current_input_srfs);
} }
else else
{ {
/* Not a top-level SRF, so recursively examine expression */ lc = list_nth_cell(context.level_input_vars, max_depth);
split_pathtarget_walker(node, &context); lfirst(lc) = list_concat(lfirst(lc), context.current_input_vars);
lc = list_nth_cell(context.level_input_srfs, max_depth);
lfirst(lc) = list_concat(lfirst(lc), context.current_input_srfs);
}
/*
* Now construct the output PathTargets. The original target can be used
* as-is for the last one, but we need to construct a new SRF-free target
* representing what the preceding plan node has to emit, as well as a
* target for each intermediate ProjectSet node.
*/
*targets = *targets_contain_srfs = NIL;
prev_level_tlist = NIL;
forthree(lc1, context.level_srfs,
lc2, context.level_input_vars,
lc3, context.level_input_srfs)
{
List *level_srfs = (List *) lfirst(lc1);
PathTarget *ntarget;
if (lnext(lc1) == NULL)
{
ntarget = target;
}
else
{
ntarget = create_empty_pathtarget();
/*
* This target should actually evaluate any SRFs of the current
* level, and it needs to propagate forward any Vars needed by
* later levels, as well as SRFs computed earlier and needed by
* later levels. We rely on add_new_columns_to_pathtarget() to
* remove duplicate items. Also, for safety, make a separate copy
* of each item for each PathTarget.
*/
add_new_columns_to_pathtarget(ntarget, copyObject(level_srfs));
for_each_cell(lc, lnext(lc2))
{
List *input_vars = (List *) lfirst(lc);
add_new_columns_to_pathtarget(ntarget, copyObject(input_vars));
}
for_each_cell(lc, lnext(lc3))
{
List *input_srfs = (List *) lfirst(lc);
ListCell *lcx;
foreach(lcx, input_srfs)
{
Node *srf = (Node *) lfirst(lcx);
if (list_member(prev_level_tlist, srf))
add_new_column_to_pathtarget(ntarget, copyObject(srf));
}
}
set_pathtarget_cost_width(root, ntarget);
}
/*
* Add current target and does-it-compute-SRFs flag to output lists.
*/
*targets = lappend(*targets, ntarget);
*targets_contain_srfs = lappend_int(*targets_contain_srfs,
(level_srfs != NIL));
/* Remember this level's output for next pass */
prev_level_tlist = ntarget->exprs;
} }
} }
/* /*
* Prepend current target and associated flag to output lists. * Recursively examine expressions for split_pathtarget_at_srfs.
*
* Note we make no effort here to prevent duplicate entries in the output
* lists. Duplicates will be gotten rid of later.
*/ */
*targets = lcons(target, *targets);
*targets_contain_srfs = lcons_int(target_contains_srfs,
*targets_contain_srfs);
/*
* Done if we found no SRFs anywhere in this target; the tentative
* tlist we built for the next level can be discarded.
*/
if (!target_contains_srfs && !context.nextlevel_contains_srfs)
break;
/*
* Else build the next PathTarget down, and loop back to process it.
* Copy the subexpressions to make sure PathTargets don't share
* substructure (might be unnecessary, but be safe); and drop any
* duplicate entries in the sub-targetlist.
*/
target = create_empty_pathtarget();
add_new_columns_to_pathtarget(target,
(List *) copyObject(context.nextlevel_tlist));
set_pathtarget_cost_width(root, target);
}
}
/* Recursively examine expressions for split_pathtarget_at_srfs */
static bool static bool
split_pathtarget_walker(Node *node, split_pathtarget_context *context) split_pathtarget_walker(Node *node, split_pathtarget_context *context)
{ {
if (node == NULL) if (node == NULL)
return false; return false;
/*
* A subexpression that matches an expression already computed in
* input_target can be treated like a Var (which indeed it will be after
* setrefs.c gets done with it), even if it's actually a SRF. Record it
* as being needed for the current expression, and ignore any
* substructure.
*/
if (list_member(context->input_target_exprs, node))
{
context->current_input_vars = lappend(context->current_input_vars,
node);
return false;
}
/*
* Vars and Var-like constructs are expected to be gotten from the input,
* too. We assume that these constructs cannot contain any SRFs (if one
* does, there will be an executor failure from a misplaced SRF).
*/
if (IsA(node, Var) || if (IsA(node, Var) ||
IsA(node, PlaceHolderVar) || IsA(node, PlaceHolderVar) ||
IsA(node, Aggref) || IsA(node, Aggref) ||
IsA(node, GroupingFunc) || IsA(node, GroupingFunc) ||
IsA(node, WindowFunc)) IsA(node, WindowFunc))
{ {
/* context->current_input_vars = lappend(context->current_input_vars,
* Pass these items down to the child plan level for evaluation. node);
*
* We assume that these constructs cannot contain any SRFs (if one
* does, there will be an executor failure from a misplaced SRF).
*/
context->nextlevel_tlist = lappend(context->nextlevel_tlist, node);
/* Having done that, we need not examine their sub-structure */
return false; return false;
} }
else if ((IsA(node, FuncExpr) &&
((FuncExpr *) node)->funcretset) || /*
(IsA(node, OpExpr) && * If it's a SRF, recursively examine its inputs, determine its level, and
((OpExpr *) node)->opretset)) * make appropriate entries in the output lists.
*/
if (IS_SRF_CALL(node))
{ {
/* List *save_input_vars = context->current_input_vars;
* Pass SRFs down to the child plan level for evaluation, and mark List *save_input_srfs = context->current_input_srfs;
* that it contains SRFs. (We are not at top level of our own tlist, int save_current_depth = context->current_depth;
* else this would have been picked up by split_pathtarget_at_srfs.) int srf_depth;
*/ ListCell *lc;
context->nextlevel_tlist = lappend(context->nextlevel_tlist, node);
context->nextlevel_contains_srfs = true;
/* Inputs to the SRF need not be considered here, so we're done */ context->current_input_vars = NIL;
context->current_input_srfs = NIL;
context->current_depth = 0;
(void) expression_tree_walker(node, split_pathtarget_walker,
(void *) context);
/* Depth is one more than any SRF below it */
srf_depth = context->current_depth + 1;
/* If new record depth, initialize another level of output lists */
if (srf_depth >= list_length(context->level_srfs))
{
context->level_srfs = lappend(context->level_srfs, NIL);
context->level_input_vars = lappend(context->level_input_vars, NIL);
context->level_input_srfs = lappend(context->level_input_srfs, NIL);
}
/* Record this SRF as needing to be evaluated at appropriate level */
lc = list_nth_cell(context->level_srfs, srf_depth);
lfirst(lc) = lappend(lfirst(lc), node);
/* Record its inputs as being needed at the same level */
lc = list_nth_cell(context->level_input_vars, srf_depth);
lfirst(lc) = list_concat(lfirst(lc), context->current_input_vars);
lc = list_nth_cell(context->level_input_srfs, srf_depth);
lfirst(lc) = list_concat(lfirst(lc), context->current_input_srfs);
/*
* Restore caller-level state and update it for presence of this SRF.
* Notice we report the SRF itself as being needed for evaluation of
* surrounding expression.
*/
context->current_input_vars = save_input_vars;
context->current_input_srfs = lappend(save_input_srfs, node);
context->current_depth = Max(save_current_depth, srf_depth);
/* We're done here */
return false; return false;
} }
/* /*
* Otherwise, the node is evaluatable within the current PathTarget, so * Otherwise, the node is a scalar (non-set) expression, so recurse to
* recurse to examine its inputs. * examine its inputs.
*/ */
return expression_tree_walker(node, split_pathtarget_walker, return expression_tree_walker(node, split_pathtarget_walker,
(void *) context); (void *) context);

2
src/test/regress/expected/subselect.out
View File

@ -853,7 +853,7 @@ select * from int4_tbl o where (f1, f1) in
-> Result -> Result
Output: i.f1, ((generate_series(1, 2)) / 10) Output: i.f1, ((generate_series(1, 2)) / 10)
-> ProjectSet -> ProjectSet
Output: i.f1, generate_series(1, 2) Output: generate_series(1, 2), i.f1
-> HashAggregate -> HashAggregate
Output: i.f1 Output: i.f1
Group Key: i.f1 Group Key: i.f1

86
src/test/regress/expected/tsrf.out
View File

@ -53,6 +53,29 @@ SELECT generate_series(generate_series(1,3), generate_series(2, 4));
4 4
(6 rows) (6 rows)
-- check proper nesting of SRFs in different expressions
explain (verbose, costs off)
SELECT generate_series(1, generate_series(1, 3)), generate_series(2, 4);
QUERY PLAN
--------------------------------------------------------------------------------
ProjectSet
Output: generate_series(1, (generate_series(1, 3))), (generate_series(2, 4))
-> ProjectSet
Output: generate_series(1, 3), generate_series(2, 4)
-> Result
(5 rows)
SELECT generate_series(1, generate_series(1, 3)), generate_series(2, 4);
generate_series | generate_series
-----------------+-----------------
1 | 2
1 | 3
2 | 3
1 | 4
2 | 4
3 | 4
(6 rows)
CREATE TABLE few(id int, dataa text, datab text); CREATE TABLE few(id int, dataa text, datab text);
INSERT INTO few VALUES(1, 'a', 'foo'),(2, 'a', 'bar'),(3, 'b', 'bar'); INSERT INTO few VALUES(1, 'a', 'foo'),(2, 'a', 'bar'),(3, 'b', 'bar');
-- SRF output order of sorting is maintained, if SRF is not referenced -- SRF output order of sorting is maintained, if SRF is not referenced
@ -518,6 +541,69 @@ SELECT |@|ARRAY[1,2,3];
3 3
(3 rows) (3 rows)
-- Some fun cases involving duplicate SRF calls
explain (verbose, costs off)
select generate_series(1,3) as x, generate_series(1,3) + 1 as xp1;
QUERY PLAN
------------------------------------------------------------------
Result
Output: (generate_series(1, 3)), ((generate_series(1, 3)) + 1)
-> ProjectSet
Output: generate_series(1, 3)
-> Result
(5 rows)
select generate_series(1,3) as x, generate_series(1,3) + 1 as xp1;
x | xp1
---+-----
1 | 2
2 | 3
3 | 4
(3 rows)
explain (verbose, costs off)
select generate_series(1,3)+1 order by generate_series(1,3);
QUERY PLAN
------------------------------------------------------------------------
Sort
Output: (((generate_series(1, 3)) + 1)), (generate_series(1, 3))
Sort Key: (generate_series(1, 3))
-> Result
Output: ((generate_series(1, 3)) + 1), (generate_series(1, 3))
-> ProjectSet
Output: generate_series(1, 3)
-> Result
(8 rows)
select generate_series(1,3)+1 order by generate_series(1,3);
?column?
----------
2
3
4
(3 rows)
-- Check that SRFs of same nesting level run in lockstep
explain (verbose, costs off)
select generate_series(1,3) as x, generate_series(3,6) + 1 as y;
QUERY PLAN
------------------------------------------------------------------
Result
Output: (generate_series(1, 3)), ((generate_series(3, 6)) + 1)
-> ProjectSet
Output: generate_series(1, 3), generate_series(3, 6)
-> Result
(5 rows)
select generate_series(1,3) as x, generate_series(3,6) + 1 as y;
x | y
---+---
1 | 4
2 | 5
3 | 6
| 7
(4 rows)
-- Clean up -- Clean up
DROP TABLE few; DROP TABLE few;
DROP TABLE fewmore; DROP TABLE fewmore;

18
src/test/regress/sql/tsrf.sql
View File

@ -17,6 +17,11 @@ SELECT generate_series(1, generate_series(1, 3));
-- srf, with two SRF arguments -- srf, with two SRF arguments
SELECT generate_series(generate_series(1,3), generate_series(2, 4)); SELECT generate_series(generate_series(1,3), generate_series(2, 4));
-- check proper nesting of SRFs in different expressions
explain (verbose, costs off)
SELECT generate_series(1, generate_series(1, 3)), generate_series(2, 4);
SELECT generate_series(1, generate_series(1, 3)), generate_series(2, 4);
CREATE TABLE few(id int, dataa text, datab text); CREATE TABLE few(id int, dataa text, datab text);
INSERT INTO few VALUES(1, 'a', 'foo'),(2, 'a', 'bar'),(3, 'b', 'bar'); INSERT INTO few VALUES(1, 'a', 'foo'),(2, 'a', 'bar'),(3, 'b', 'bar');
@ -129,6 +134,19 @@ SELECT (SELECT generate_series(1,3) LIMIT 1 OFFSET g.i) FROM generate_series(0,3
CREATE OPERATOR |@| (PROCEDURE = unnest, RIGHTARG = ANYARRAY); CREATE OPERATOR |@| (PROCEDURE = unnest, RIGHTARG = ANYARRAY);
SELECT |@|ARRAY[1,2,3]; SELECT |@|ARRAY[1,2,3];
-- Some fun cases involving duplicate SRF calls
explain (verbose, costs off)
select generate_series(1,3) as x, generate_series(1,3) + 1 as xp1;
select generate_series(1,3) as x, generate_series(1,3) + 1 as xp1;
explain (verbose, costs off)
select generate_series(1,3)+1 order by generate_series(1,3);
select generate_series(1,3)+1 order by generate_series(1,3);
-- Check that SRFs of same nesting level run in lockstep
explain (verbose, costs off)
select generate_series(1,3) as x, generate_series(3,6) + 1 as y;
select generate_series(1,3) as x, generate_series(3,6) + 1 as y;
-- Clean up -- Clean up
DROP TABLE few; DROP TABLE few;
DROP TABLE fewmore; DROP TABLE fewmore;