diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c index 880b0ec6846..34eda88099b 100644 --- a/src/backend/nodes/nodeFuncs.c +++ b/src/backend/nodes/nodeFuncs.c @@ -2378,59 +2378,74 @@ range_table_walker(List *rtable, foreach(rt, rtable) { - RangeTblEntry *rte = (RangeTblEntry *) lfirst(rt); + RangeTblEntry *rte = lfirst_node(RangeTblEntry, rt); - /* - * Walkers might need to examine the RTE node itself either before or - * after visiting its contents (or, conceivably, both). Note that if - * you specify neither flag, the walker won't visit the RTE at all. - */ - if (flags & QTW_EXAMINE_RTES_BEFORE) - if (walker(rte, context)) - return true; + if (range_table_entry_walker(rte, walker, context, flags)) + return true; + } + return false; +} - switch (rte->rtekind) - { - case RTE_RELATION: - if (walker(rte->tablesample, context)) - return true; - break; - case RTE_SUBQUERY: - if (!(flags & QTW_IGNORE_RT_SUBQUERIES)) - if (walker(rte->subquery, context)) - return true; - break; - case RTE_JOIN: - if (!(flags & QTW_IGNORE_JOINALIASES)) - if (walker(rte->joinaliasvars, context)) - return true; - break; - case RTE_FUNCTION: - if (walker(rte->functions, context)) - return true; - break; - case RTE_TABLEFUNC: - if (walker(rte->tablefunc, context)) - return true; - break; - case RTE_VALUES: - if (walker(rte->values_lists, context)) - return true; - break; - case RTE_CTE: - case RTE_NAMEDTUPLESTORE: - case RTE_RESULT: - /* nothing to do */ - break; - } - - if (walker(rte->securityQuals, context)) +/* + * Some callers even want to scan the expressions in individual RTEs. + */ +bool +range_table_entry_walker(RangeTblEntry *rte, + bool (*walker) (), + void *context, + int flags) +{ + /* + * Walkers might need to examine the RTE node itself either before or + * after visiting its contents (or, conceivably, both). Note that if you + * specify neither flag, the walker won't be called on the RTE at all. + */ + if (flags & QTW_EXAMINE_RTES_BEFORE) + if (walker(rte, context)) return true; - if (flags & QTW_EXAMINE_RTES_AFTER) - if (walker(rte, context)) + switch (rte->rtekind) + { + case RTE_RELATION: + if (walker(rte->tablesample, context)) return true; + break; + case RTE_SUBQUERY: + if (!(flags & QTW_IGNORE_RT_SUBQUERIES)) + if (walker(rte->subquery, context)) + return true; + break; + case RTE_JOIN: + if (!(flags & QTW_IGNORE_JOINALIASES)) + if (walker(rte->joinaliasvars, context)) + return true; + break; + case RTE_FUNCTION: + if (walker(rte->functions, context)) + return true; + break; + case RTE_TABLEFUNC: + if (walker(rte->tablefunc, context)) + return true; + break; + case RTE_VALUES: + if (walker(rte->values_lists, context)) + return true; + break; + case RTE_CTE: + case RTE_NAMEDTUPLESTORE: + case RTE_RESULT: + /* nothing to do */ + break; } + + if (walker(rte->securityQuals, context)) + return true; + + if (flags & QTW_EXAMINE_RTES_AFTER) + if (walker(rte, context)) + return true; + return false; } diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index db25bcf441f..e97c6ec7f6c 100644 --- a/src/backend/optimizer/prep/prepjointree.c +++ b/src/backend/optimizer/prep/prepjointree.c @@ -120,7 +120,9 @@ static void reduce_outer_joins_pass2(Node *jtnode, static Node *remove_useless_results_recurse(PlannerInfo *root, Node *jtnode); static int get_result_relid(PlannerInfo *root, Node *jtnode); static void remove_result_refs(PlannerInfo *root, int varno, Node *newjtloc); -static bool find_dependent_phvs(Node *node, int varno); +static bool find_dependent_phvs(PlannerInfo *root, int varno); +static bool find_dependent_phvs_in_jointree(PlannerInfo *root, + Node *node, int varno); static void substitute_phv_relids(Node *node, int varno, Relids subrelids); static void fix_append_rel_relids(List *append_rel_list, int varno, @@ -2957,9 +2959,12 @@ reduce_outer_joins_pass2(Node *jtnode, * and not remove the RTE_RESULT: there is noplace else to evaluate the * PlaceHolderVar. (That is, in such cases the RTE_RESULT *does* have output * columns.) But if the RTE_RESULT is an immediate child of an inner join, - * we can change the PlaceHolderVar's phrels so as to evaluate it at the - * inner join instead. This is OK because we really only care that PHVs are - * evaluated above or below the correct outer joins. + * we can usually change the PlaceHolderVar's phrels so as to evaluate it at + * the inner join instead. This is OK because we really only care that PHVs + * are evaluated above or below the correct outer joins. We can't, however, + * postpone the evaluation of a PHV to above where it is used; so there are + * some checks below on whether output PHVs are laterally referenced in the + * other join input rel(s). * * We used to try to do this work as part of pull_up_subqueries() where the * potentially-optimizable cases get introduced; but it's way simpler, and @@ -3021,8 +3026,11 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode) /* * We can drop RTE_RESULT rels from the fromlist so long as at least * one child remains, since joining to a one-row table changes - * nothing. The easiest way to mechanize this rule is to modify the - * list in-place. + * nothing. (But we can't drop a RTE_RESULT that computes PHV(s) that + * are needed by some sibling. The cleanup transformation below would + * reassign the PHVs to be computed at the join, which is too late for + * the sibling's use.) The easiest way to mechanize this rule is to + * modify the list in-place. */ foreach(cell, f->fromlist) { @@ -3035,12 +3043,14 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode) lfirst(cell) = child; /* - * If it's an RTE_RESULT with at least one sibling, we can drop - * it. We don't yet know what the inner join's final relid set - * will be, so postpone cleanup of PHVs etc till after this loop. + * If it's an RTE_RESULT with at least one sibling, and no sibling + * references dependent PHVs, we can drop it. We don't yet know + * what the inner join's final relid set will be, so postpone + * cleanup of PHVs etc till after this loop. */ if (list_length(f->fromlist) > 1 && - (varno = get_result_relid(root, child)) != 0) + (varno = get_result_relid(root, child)) != 0 && + !find_dependent_phvs_in_jointree(root, (Node *) f, varno)) { f->fromlist = foreach_delete_current(f->fromlist, cell); result_relids = bms_add_member(result_relids, varno); @@ -3091,8 +3101,18 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode) * the join with a FromExpr with just the other side; and if * the qual is empty (JOIN ON TRUE) then we can omit the * FromExpr as well. + * + * Just as in the FromExpr case, we can't simplify if the + * other input rel references any PHVs that are marked as to + * be evaluated at the RTE_RESULT rel, because we can't + * postpone their evaluation in that case. But we only have + * to check this in cases where it's syntactically legal for + * the other input to have a LATERAL reference to the + * RTE_RESULT rel. Only RHSes of inner and left joins are + * allowed to have such refs. */ - if ((varno = get_result_relid(root, j->larg)) != 0) + if ((varno = get_result_relid(root, j->larg)) != 0 && + !find_dependent_phvs_in_jointree(root, j->rarg, varno)) { remove_result_refs(root, varno, j->rarg); if (j->quals) @@ -3121,6 +3141,8 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode) * strength-reduced to a plain inner join, since each LHS row * necessarily has exactly one join partner. So we can always * discard the RHS, much as in the JOIN_INNER case above. + * (Again, the LHS could not contain a lateral reference to + * the RHS.) * * Otherwise, it's still true that each LHS row should be * returned exactly once, and since the RHS returns no columns @@ -3131,7 +3153,7 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode) */ if ((varno = get_result_relid(root, j->rarg)) != 0 && (j->quals == NULL || - !find_dependent_phvs((Node *) root->parse, varno))) + !find_dependent_phvs(root, varno))) { remove_result_refs(root, varno, j->larg); jtnode = j->larg; @@ -3141,7 +3163,7 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode) /* Mirror-image of the JOIN_LEFT case */ if ((varno = get_result_relid(root, j->larg)) != 0 && (j->quals == NULL || - !find_dependent_phvs((Node *) root->parse, varno))) + !find_dependent_phvs(root, varno))) { remove_result_refs(root, varno, j->rarg); jtnode = j->rarg; @@ -3162,7 +3184,7 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode) */ if ((varno = get_result_relid(root, j->rarg)) != 0) { - Assert(!find_dependent_phvs((Node *) root->parse, varno)); + Assert(!find_dependent_phvs(root, varno)); remove_result_refs(root, varno, j->larg); if (j->quals) jtnode = (Node *) @@ -3246,6 +3268,12 @@ remove_result_refs(PlannerInfo *root, int varno, Node *newjtloc) /* * find_dependent_phvs - are there any PlaceHolderVars whose relids are * exactly the given varno? + * + * find_dependent_phvs should be used when we want to see if there are + * any such PHVs anywhere in the Query. Another use-case is to see if + * a subtree of the join tree contains such PHVs; but for that, we have + * to look not only at the join tree nodes themselves but at the + * referenced RTEs. For that, use find_dependent_phvs_in_jointree. */ typedef struct @@ -3292,20 +3320,65 @@ find_dependent_phvs_walker(Node *node, } static bool -find_dependent_phvs(Node *node, int varno) +find_dependent_phvs(PlannerInfo *root, int varno) { find_dependent_phvs_context context; + /* If there are no PHVs anywhere, we needn't work hard */ + if (root->glob->lastPHId == 0) + return false; + + context.relids = bms_make_singleton(varno); + context.sublevels_up = 0; + + return query_tree_walker(root->parse, + find_dependent_phvs_walker, + (void *) &context, + 0); +} + +static bool +find_dependent_phvs_in_jointree(PlannerInfo *root, Node *node, int varno) +{ + find_dependent_phvs_context context; + Relids subrelids; + int relid; + + /* If there are no PHVs anywhere, we needn't work hard */ + if (root->glob->lastPHId == 0) + return false; + context.relids = bms_make_singleton(varno); context.sublevels_up = 0; /* - * Must be prepared to start with a Query or a bare expression tree. + * See if the jointree fragment itself contains references (in join quals) */ - return query_or_expression_tree_walker(node, - find_dependent_phvs_walker, - (void *) &context, - 0); + if (find_dependent_phvs_walker(node, &context)) + return true; + + /* + * Otherwise, identify the set of referenced RTEs (we can ignore joins, + * since they should be flattened already, so their join alias lists no + * longer matter), and tediously check each RTE. We can ignore RTEs that + * are not marked LATERAL, though, since they couldn't possibly contain + * any cross-references to other RTEs. + */ + subrelids = get_relids_in_jointree(node, false); + relid = -1; + while ((relid = bms_next_member(subrelids, relid)) >= 0) + { + RangeTblEntry *rte = rt_fetch(relid, root->parse->rtable); + + if (rte->lateral && + range_table_entry_walker(rte, + find_dependent_phvs_walker, + (void *) &context, + 0)) + return true; + } + + return false; } /* diff --git a/src/include/nodes/nodeFuncs.h b/src/include/nodes/nodeFuncs.h index 4b5408fa9b1..1e55683a95a 100644 --- a/src/include/nodes/nodeFuncs.h +++ b/src/include/nodes/nodeFuncs.h @@ -141,6 +141,9 @@ extern bool range_table_walker(List *rtable, bool (*walker) (), extern List *range_table_mutator(List *rtable, Node *(*mutator) (), void *context, int flags); +extern bool range_table_entry_walker(RangeTblEntry *rte, bool (*walker) (), + void *context, int flags); + extern bool query_or_expression_tree_walker(Node *node, bool (*walker) (), void *context, int flags); extern Node *query_or_expression_tree_mutator(Node *node, Node *(*mutator) (), diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index 1a08e6321d7..761376b0079 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -3242,6 +3242,32 @@ where t1.unique2 < 42 and t1.stringu1 > t2.stringu2; 11 | WFAAAA | 3 | LKIAAA (1 row) +-- Here's a variant that we can't fold too aggressively, though, +-- or we end up with noplace to evaluate the lateral PHV +explain (verbose, costs off) +select * from + (select 1 as x) ss1 left join (select 2 as y) ss2 on (true), + lateral (select ss2.y as z limit 1) ss3; + QUERY PLAN +--------------------------- + Nested Loop + Output: 1, (2), ((2)) + -> Result + Output: 2 + -> Limit + Output: ((2)) + -> Result + Output: (2) +(8 rows) + +select * from + (select 1 as x) ss1 left join (select 2 as y) ss2 on (true), + lateral (select ss2.y as z limit 1) ss3; + x | y | z +---+---+--- + 1 | 2 | 2 +(1 row) + -- -- test inlining of immutable functions -- diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index 57481d04117..5fc66173690 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -1018,6 +1018,16 @@ select t1.unique2, t1.stringu1, t2.unique1, t2.stringu2 from on (subq1.y1 = t2.unique1) where t1.unique2 < 42 and t1.stringu1 > t2.stringu2; +-- Here's a variant that we can't fold too aggressively, though, +-- or we end up with noplace to evaluate the lateral PHV +explain (verbose, costs off) +select * from + (select 1 as x) ss1 left join (select 2 as y) ss2 on (true), + lateral (select ss2.y as z limit 1) ss3; +select * from + (select 1 as x) ss1 left join (select 2 as y) ss2 on (true), + lateral (select ss2.y as z limit 1) ss3; + -- -- test inlining of immutable functions --