pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #18305: Unexpected error: "WindowFunc not found in subplan target lists" triggered by subqueries
15+ messages / 4 participants
[nested] [flat]
Commits from this thread:
2024-05-05 7d2c7f08d9 Fix query pullup issue with WindowClause runCondition
* BUG #18305: Unexpected error: "WindowFunc not found in subplan target lists" triggered by subqueries
@ 2024-01-22 12:15 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 15+ messages in thread
From: PG Bug reporting form @ 2024-01-22 12:15 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: zuming.jiang@inf.ethz.ch
The following bug has been logged on the website:
Bug reference: 18305
Logged by: Zuming Jiang
Email address: zuming.jiang@inf.ethz.ch
PostgreSQL version: 16.1
Operating system: Ubuntu 20.04
Description:
My fuzzer finds a bug in Postgres 17devel, which triggers an unexpected
error "ERROR: WindowFunc not found in subplan target lists".
--- Set up database ---
create table exeet_t3 (pkey int4);
create view exeet_t8 as
select
ntile(exeet_subq_0.c_0) over () as c_0
from
(select (select pkey from exeet_t3 order by pkey limit 1 offset 6) as
c_0) as exeet_subq_0;
The fuzzer generates a test case:
--- Test case ---
select
1 as c_1
from
exeet_t8 as exeet_ref_17
where exeet_ref_17.c_0 < 0;
--- Expected behavior ---
The test case should not trigger any error.
--- Actual behavior ---
The test case trigger an error:
ERROR: WindowFunc not found in subplan target lists
--- Postgres version ---
Github commit: b0f0a9432d0b6f53634a96715f2666f6d4ea25a1
Version: PostgreSQL 17devel on x86_64-pc-linux-gnu, compiled by gcc
(Ubuntu
9.4.0-1ubuntu1~20.04.2) 9.4.0, 64-bit
--- Platform information ---
Platform: Ubuntu 20.04
Kernel: Linux 5.4.0-147-generic
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #18305: Unexpected error: "WindowFunc not found in subplan target lists" triggered by subqueries
@ 2024-01-22 15:44 Tom Lane <tgl@sss.pgh.pa.us>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 15+ messages in thread
From: Tom Lane @ 2024-01-22 15:44 UTC (permalink / raw)
To: zuming.jiang@inf.ethz.ch; +Cc: David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org
PG Bug reporting form <noreply@postgresql.org> writes:
> My fuzzer finds a bug in Postgres 17devel, which triggers an unexpected
> error "ERROR: WindowFunc not found in subplan target lists".
> create table exeet_t3 (pkey int4);
> create view exeet_t8 as
> select
> ntile(exeet_subq_0.c_0) over () as c_0
> from
> (select (select pkey from exeet_t3 order by pkey limit 1 offset 6) as
> c_0) as exeet_subq_0;
> select
> 1 as c_1
> from
> exeet_t8 as exeet_ref_17
> where exeet_ref_17.c_0 < 0;
> ERROR: WindowFunc not found in subplan target lists
Thanks for the report! Bisecting shows that this broke at
456fa635a909ee36f73ca84d340521bd730f265f is the first bad commit
commit 456fa635a909ee36f73ca84d340521bd730f265f
Author: David Rowley <drowley@postgresql.org>
Date: Fri Jan 27 16:08:41 2023 +1300
Teach planner about more monotonic window functions
9d9c02ccd introduced runConditions for window functions to allow
monotonic window function evaluation to be made more efficient when the
window function value went beyond some value that it would never go back
from due to its monotonic nature. That commit added prosupport functions
to inform the planner that row_number(), rank(), dense_rank() and some
forms of count(*) were monotonic. Here we add support for ntile(),
cume_dist() and percent_rank().
So I'm betting there's something wrong with the ntile support
function, but I've not dug deeper.
regards, tom lane
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #18305: Unexpected error: "WindowFunc not found in subplan target lists" triggered by subqueries
@ 2024-01-23 10:41 Richard Guo <guofenglinux@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 15+ messages in thread
From: Richard Guo @ 2024-01-23 10:41 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: zuming.jiang@inf.ethz.ch, David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org
On Mon, Jan 22, 2024 at 11:44 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> PG Bug reporting form <noreply@postgresql.org> writes:
> > My fuzzer finds a bug in Postgres 17devel, which triggers an unexpected
> > error "ERROR: WindowFunc not found in subplan target lists".
>
> > create table exeet_t3 (pkey int4);
> > create view exeet_t8 as
> > select
> > ntile(exeet_subq_0.c_0) over () as c_0
> > from
> > (select (select pkey from exeet_t3 order by pkey limit 1 offset 6) as
> > c_0) as exeet_subq_0;
>
> > select
> > 1 as c_1
> > from
> > exeet_t8 as exeet_ref_17
> > where exeet_ref_17.c_0 < 0;
> > ERROR: WindowFunc not found in subplan target lists
>
> Thanks for the report! Bisecting shows that this broke at
>
> 456fa635a909ee36f73ca84d340521bd730f265f is the first bad commit
> commit 456fa635a909ee36f73ca84d340521bd730f265f
> Author: David Rowley <drowley@postgresql.org>
> Date: Fri Jan 27 16:08:41 2023 +1300
>
> Teach planner about more monotonic window functions
>
> 9d9c02ccd introduced runConditions for window functions to allow
> monotonic window function evaluation to be made more efficient when the
> window function value went beyond some value that it would never go
> back
> from due to its monotonic nature. That commit added prosupport
> functions
> to inform the planner that row_number(), rank(), dense_rank() and some
> forms of count(*) were monotonic. Here we add support for ntile(),
> cume_dist() and percent_rank().
>
> So I'm betting there's something wrong with the ntile support
> function, but I've not dug deeper.
Here is a simplified repro query:
select 1 from
(select ntile(s1.x) over () as c
from (select (select 1) as x) as s1) s
where s.c = 1;
ERROR: WindowFunc not found in subplan target lists
I think the problem is that in find_window_run_conditions() we fail to
check thoroughly whether the WindowFunc contains any potential subplan
nodes. We do have the check:
/* can't use it if there are subplans in the WindowFunc */
if (contain_subplans((Node *) wfunc))
return false;
... and the bug being reported here indicates that the current check is
insufficient, because a Var node inside the WindowFunc could potentially
belong to a subquery and reference a SubLink expression.
In the query above, the WindowFunc's argument, 's1.x', is actually an
EXPR_SUBLINK sublink. After we've pulled up subquery 's1', the
arguments of the two WindowFuncs (one in the target list of subquery
's', the other in the WindowClause's runCondition of subquery 's') would
be replaced with SubLink nodes. And then after we've converted SubLink
nodes to SubPlans, these two SubLink nodes would be replaced with
Params, but with different paramids.
Hmm, is there a way to detect in find_window_run_conditions() whether
the WindowFunc's argument references a SubLink node of a subquery that
hasn't been pulled up?
Thanks
Richard
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #18305: Unexpected error: "WindowFunc not found in subplan target lists" triggered by subqueries
@ 2024-01-23 13:52 Tom Lane <tgl@sss.pgh.pa.us>
parent: Richard Guo <guofenglinux@gmail.com>
1 sibling, 0 replies; 15+ messages in thread
From: Tom Lane @ 2024-01-23 13:52 UTC (permalink / raw)
To: Richard Guo <guofenglinux@gmail.com>; +Cc: zuming.jiang@inf.ethz.ch, David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org
Richard Guo <guofenglinux@gmail.com> writes:
> Hmm, is there a way to detect in find_window_run_conditions() whether
> the WindowFunc's argument references a SubLink node of a subquery that
> hasn't been pulled up?
contain_subplans will recognize both SubLinks and SubPlans,
so there must be some other factor here. (I didn't look yet.)
regards, tom lane
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #18305: Unexpected error: "WindowFunc not found in subplan target lists" triggered by subqueries
@ 2024-01-23 19:34 Tom Lane <tgl@sss.pgh.pa.us>
parent: Richard Guo <guofenglinux@gmail.com>
1 sibling, 1 reply; 15+ messages in thread
From: Tom Lane @ 2024-01-23 19:34 UTC (permalink / raw)
To: Richard Guo <guofenglinux@gmail.com>; +Cc: zuming.jiang@inf.ethz.ch, David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org
Richard Guo <guofenglinux@gmail.com> writes:
> I think the problem is that in find_window_run_conditions() we fail to
> check thoroughly whether the WindowFunc contains any potential subplan
> nodes. We do have the check:
> /* can't use it if there are subplans in the WindowFunc */
> if (contain_subplans((Node *) wfunc))
> return false;
> ... and the bug being reported here indicates that the current check is
> insufficient, because a Var node inside the WindowFunc could potentially
> belong to a subquery and reference a SubLink expression.
I dug through this, and my conclusion is that the entire "run
condition" optimization is seriously broken, because the tree
manipulations here are many bricks shy of a load. There are
two big problems:
1. The subquery hasn't yet been through any preprocessing. Thus,
we've not yet done subquery pullup within it, which is why the
contain_subplans call fails to detect a problem: the WindowFunc's arg
is just a Var. Later, pullup of the lower subquery will replace that
Var with a SubLink, which eventually gets replaced with a Param
referencing an initplan's output. However, run condition optimization
caused us to put a second copy of the WindowFunc into the
runCondition, and that copy gets its own copy of the SubLink, which
eventually results in a different Param, leading to the observed
failure. (BTW, it scares the heck out of me that we just link the
WindowFunc expr into wclause->runCondition without making a copy of
it. That could be safe if the planner never scribbles on its input,
but that ain't so.)
2. We are pushing the "otherexpr" from the upper-level query
(which *has* been through preprocessing) into the unprocessed
child query. This makes me acutely uncomfortable:
A. Almost certainly, this fails if otherexpr contains a subplan.
B. Since nothing is done about levelsup adjustment, it will
fail if anything in that tree includes a levelsup field.
Fortunately is_pseudo_constant_clause will reject Vars,
but those aren't the only things with levelsup. I wonder
if it's possible to get an uplevel Aggref into a subquery's
restrictlist, in particular.
C. Again, failure to make a copy of the expression tree seems
pretty dangerous.
D. We're relying on repeat preprocessing of the otherexpr to
do no harm. Maybe that's OK, but it's not great.
The first three things are not hard to fix, but they're not being
taken care of today.
We could work around point 1 by refusing to pull up subqueries that
contain sublinks in their targetlists, but that would be a pretty big
change (and, probably, a pessimization of some queries). I do not
consider run-condition optimization to justify that. Moreover
I'm not sure that sublinks are the only thing that could get
mutated to a different state in the runCondition than in the main
tree.
I think the only real way to prevent problems from point 1 is to stop
making a copy of the WindowFunc expr. We need some other way to refer
to the WindowFunc's value in the runCondition tree. Maybe a generated
Param would serve?
regards, tom lane
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #18305: Unexpected error: "WindowFunc not found in subplan target lists" triggered by subqueries
@ 2024-01-25 05:14 David Rowley <dgrowleyml@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 15+ messages in thread
From: David Rowley @ 2024-01-25 05:14 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Richard Guo <guofenglinux@gmail.com>; zuming.jiang@inf.ethz.ch; pgsql-bugs@lists.postgresql.org
On Wed, 24 Jan 2024 at 08:34, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> We could work around point 1 by refusing to pull up subqueries that
> contain sublinks in their targetlists, but that would be a pretty big
> change (and, probably, a pessimization of some queries). I do not
> consider run-condition optimization to justify that. Moreover
> I'm not sure that sublinks are the only thing that could get
> mutated to a different state in the runCondition than in the main
> tree.
>
> I think the only real way to prevent problems from point 1 is to stop
> making a copy of the WindowFunc expr. We need some other way to refer
> to the WindowFunc's value in the runCondition tree. Maybe a generated
> Param would serve?
I'm wondering if it was wrong to put the runCondition field in
WindowClause. Maybe it should be in WindowFunc instead...
Really the OpExpr that's created in find_window_run_conditions() with
the WindowFunc as an arg is there to show the Run Condition in
EXPLAIN. This is what's later stored in WindowAgg.runConditionOrig.
What we pass to ExecQual in nodeWindowAgg.c is a version of the OpExpr
with the WindowFunc replaced with a Var so that ExecQual properly
fetches the just-calculated WindowFunc return value from the slot. We
don't want to leave the WindowFunc in the runCondition as ExecQual
would go and evaluate it.
If WindowFunc allowed a list of a new struct called WindowRunCondition
with fields "otherarg", "opno", "collation", "wfunc_left" then we
could construct the OpExpr later either in createplan.c or setrefs.c.
The EXPLAIN version of that OpExpr could have the WindowFunc and the
non-EXPLAIN version would have the Var.
Sounds a bit invasive for back branches, but wondering if we couldn't
just modify window_ntile_support() to reject any ntile args other than
Consts. count(*), row_number(), rank(), dense_rank(), percent_rank()
and percent_rank() all can't suffer from this issue as they don't have
an argument. count(expr) would need to have something done to stop
the same issue from occurring. Maybe int8inc_support() could just set
req->monotonic = MONOTONICFUNC_NONE if the req->window_func has an
arg, effectively disabling the optimisation for count(expr).
David
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #18305: Unexpected error: "WindowFunc not found in subplan target lists" triggered by subqueries
@ 2024-01-26 12:02 David Rowley <dgrowleyml@gmail.com>
parent: David Rowley <dgrowleyml@gmail.com>
1 sibling, 2 replies; 15+ messages in thread
From: David Rowley @ 2024-01-26 12:02 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Richard Guo <guofenglinux@gmail.com>; zuming.jiang@inf.ethz.ch; pgsql-bugs@lists.postgresql.org
On Thu, 25 Jan 2024 at 18:14, David Rowley <dgrowleyml@gmail.com> wrote:
>
> On Wed, 24 Jan 2024 at 08:34, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > We could work around point 1 by refusing to pull up subqueries that
> > contain sublinks in their targetlists, but that would be a pretty big
> > change (and, probably, a pessimization of some queries). I do not
> > consider run-condition optimization to justify that. Moreover
> > I'm not sure that sublinks are the only thing that could get
> > mutated to a different state in the runCondition than in the main
> > tree.
> >
> > I think the only real way to prevent problems from point 1 is to stop
> > making a copy of the WindowFunc expr. We need some other way to refer
> > to the WindowFunc's value in the runCondition tree. Maybe a generated
> > Param would serve?
>
> I'm wondering if it was wrong to put the runCondition field in
> WindowClause. Maybe it should be in WindowFunc instead...
>
> Really the OpExpr that's created in find_window_run_conditions() with
> the WindowFunc as an arg is there to show the Run Condition in
> EXPLAIN. This is what's later stored in WindowAgg.runConditionOrig.
> What we pass to ExecQual in nodeWindowAgg.c is a version of the OpExpr
> with the WindowFunc replaced with a Var so that ExecQual properly
> fetches the just-calculated WindowFunc return value from the slot. We
> don't want to leave the WindowFunc in the runCondition as ExecQual
> would go and evaluate it.
>
> If WindowFunc allowed a list of a new struct called WindowRunCondition
> with fields "otherarg", "opno", "collation", "wfunc_left" then we
> could construct the OpExpr later either in createplan.c or setrefs.c.
> The EXPLAIN version of that OpExpr could have the WindowFunc and the
> non-EXPLAIN version would have the Var.
Just to assist the discussion here I've drafted a patch along the
lines of the above. See attached
If you think this idea has merit I can try and turn it into something
committable for master.
> Sounds a bit invasive for back branches, but wondering if we couldn't
> just modify window_ntile_support() to reject any ntile args other than
> Consts. count(*), row_number(), rank(), dense_rank(), percent_rank()
> and percent_rank() all can't suffer from this issue as they don't have
> an argument. count(expr) would need to have something done to stop
> the same issue from occurring. Maybe int8inc_support() could just set
> req->monotonic = MONOTONICFUNC_NONE if the req->window_func has an
> arg, effectively disabling the optimisation for count(expr).
I'm still unsure about the fix for back branches but I'm open to other ideas.
David
Attachments:
poc_runcondition_fix_draft.patch (text/plain, 15.3K) download
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index e1a5bc7e95..030463cb42 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -2283,8 +2283,6 @@ expression_tree_walker_impl(Node *node,
return true;
if (WALK(wc->endOffset))
return true;
- if (WALK(wc->runCondition))
- return true;
}
break;
case T_CTECycleClause:
@@ -2629,8 +2627,6 @@ query_tree_walker_impl(Query *query,
return true;
if (WALK(wc->endOffset))
return true;
- if (WALK(wc->runCondition))
- return true;
}
}
@@ -3316,7 +3312,6 @@ expression_tree_mutator_impl(Node *node,
MUTATE(newnode->orderClause, wc->orderClause, List *);
MUTATE(newnode->startOffset, wc->startOffset, Node *);
MUTATE(newnode->endOffset, wc->endOffset, Node *);
- MUTATE(newnode->runCondition, wc->runCondition, List *);
return (Node *) newnode;
}
break;
@@ -3646,7 +3641,6 @@ query_tree_mutator_impl(Query *query,
FLATCOPY(newnode, wc, WindowClause);
MUTATE(newnode->startOffset, wc->startOffset, Node *);
MUTATE(newnode->endOffset, wc->endOffset, Node *);
- MUTATE(newnode->runCondition, wc->runCondition, List *);
resultlist = lappend(resultlist, (Node *) newnode);
}
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 84c4de488a..ae2e930931 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -2208,7 +2208,7 @@ set_dummy_rel_pathlist(RelOptInfo *rel)
* the run condition will handle all of the required filtering.
*
* Returns true if 'opexpr' was found to be useful and was added to the
- * WindowClauses runCondition. We also set *keep_original accordingly and add
+ * WindowFunc's runCondition. We also set *keep_original accordingly and add
* 'attno' to *run_cond_attrs offset by FirstLowInvalidHeapAttributeNumber.
* If the 'opexpr' cannot be used then we set *keep_original to true and
* return false.
@@ -2361,7 +2361,7 @@ find_window_run_conditions(Query *subquery, RangeTblEntry *rte, Index rti,
*keep_original = true;
runopexpr = opexpr;
- /* determine the operator to use for the runCondition qual */
+ /* determine the operator to use for the WindwFuncRunCondition */
runoperator = get_opfamily_member(opinfo->opfamily_id,
opinfo->oplefttype,
opinfo->oprighttype,
@@ -2372,27 +2372,15 @@ find_window_run_conditions(Query *subquery, RangeTblEntry *rte, Index rti,
if (runopexpr != NULL)
{
- Expr *newexpr;
+ WindowFuncRunCondition *runcond;
- /*
- * Build the qual required for the run condition keeping the
- * WindowFunc on the same side as it was originally.
- */
- if (wfunc_left)
- newexpr = make_opclause(runoperator,
- runopexpr->opresulttype,
- runopexpr->opretset, (Expr *) wfunc,
- otherexpr, runopexpr->opcollid,
- runopexpr->inputcollid);
- else
- newexpr = make_opclause(runoperator,
- runopexpr->opresulttype,
- runopexpr->opretset,
- otherexpr, (Expr *) wfunc,
- runopexpr->opcollid,
- runopexpr->inputcollid);
+ runcond = makeNode(WindowFuncRunCondition);
+ runcond->opno = runoperator;
+ runcond->inputcollid = runopexpr->inputcollid;
+ runcond->wfunc_left = wfunc_left;
+ runcond->arg = copyObject(otherexpr);
- wclause->runCondition = lappend(wclause->runCondition, newexpr);
+ wfunc->runCondition = lappend(wfunc->runCondition, runcond);
/* record that this attno was used in a run condition */
*run_cond_attrs = bms_add_member(*run_cond_attrs,
@@ -2406,9 +2394,9 @@ find_window_run_conditions(Query *subquery, RangeTblEntry *rte, Index rti,
/*
* check_and_push_window_quals
- * Check if 'clause' is a qual that can be pushed into a WindowFunc's
- * WindowClause as a 'runCondition' qual. These, when present, allow
- * some unnecessary work to be skipped during execution.
+ * Check if 'clause' is a qual that can be pushed into a WindowFunc
+ * as a 'runCondition' qual. These, when present, allow some unnecessary
+ * work to be skipped during execution.
*
* 'run_cond_attrs' will be populated with all targetlist resnos of subquery
* targets (offset by FirstLowInvalidHeapAttributeNumber) that we pushed
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 610f4a56d6..fa85f0e0ff 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -2697,7 +2697,7 @@ create_windowagg_plan(PlannerInfo *root, WindowAggPath *best_path)
wc->inRangeColl,
wc->inRangeAsc,
wc->inRangeNullsFirst,
- wc->runCondition,
+ best_path->runCondition,
best_path->qual,
best_path->topwindow,
subplan);
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 2e2458b128..35923ba5bc 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -865,9 +865,6 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
EXPRKIND_LIMIT);
wc->endOffset = preprocess_expression(root, wc->endOffset,
EXPRKIND_LIMIT);
- wc->runCondition = (List *) preprocess_expression(root,
- (Node *) wc->runCondition,
- EXPRKIND_TARGET);
}
parse->limitOffset = preprocess_expression(root, parse->limitOffset,
@@ -4474,9 +4471,11 @@ create_one_window_path(PlannerInfo *root,
{
WindowClause *wc = lfirst_node(WindowClause, l);
List *window_pathkeys;
+ List *runcondition = NIL;
int presorted_keys;
bool is_sorted;
bool topwindow;
+ ListCell *lc2;
window_pathkeys = make_pathkeys_for_window(root,
wc,
@@ -4524,7 +4523,6 @@ create_one_window_path(PlannerInfo *root,
* we do need to account for the increase in tlist width.
*/
int64 tuple_width = window_target->width;
- ListCell *lc2;
window_target = copy_pathtarget(window_target);
foreach(lc2, wflists->windowFuncs[wc->winref])
@@ -4546,17 +4544,53 @@ create_one_window_path(PlannerInfo *root,
topwindow = foreach_current_index(l) == list_length(activeWindows) - 1;
/*
- * Accumulate all of the runConditions from each intermediate
- * WindowClause. The top-level WindowAgg must pass these as a qual so
- * that it filters out unwanted tuples correctly.
+ * Collect the WindowFuncRunConditions from each WindowFunc and
+ * convert them into OpExprs
*/
- if (!topwindow)
- topqual = list_concat(topqual, wc->runCondition);
+ foreach (lc2, wflists->windowFuncs[wc->winref])
+ {
+ ListCell *lc3;
+ WindowFunc *wfunc = lfirst_node(WindowFunc, lc2);
+
+ foreach (lc3, wfunc->runCondition)
+ {
+ WindowFuncRunCondition *rc =
+ lfirst_node(WindowFuncRunCondition, lc3);
+ Expr *opexpr;
+ Expr *leftop;
+ Expr *rightop;
+
+ if (rc->wfunc_left)
+ {
+ leftop = copyObject(wfunc);
+ rightop = copyObject(rc->arg);
+ }
+ else
+ {
+ leftop = copyObject(rc->arg);
+ rightop = copyObject(wfunc);
+ }
+
+ opexpr = make_opclause(rc->opno,
+ BOOLOID,
+ false,
+ leftop,
+ rightop,
+ InvalidOid,
+ rc->inputcollid);
+
+ runcondition = lappend(runcondition, opexpr);
+
+ if (!topwindow)
+ topqual = lappend(topqual, opexpr);
+ }
+ }
path = (Path *)
create_windowagg_path(root, window_rel, path, window_target,
wflists->windowFuncs[wc->winref],
- wc, topwindow ? topqual : NIL, topwindow);
+ runcondition, wc,
+ topwindow ? topqual : NIL, topwindow);
}
add_path(window_rel, path);
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index aa83dd3636..0237ff57d5 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -2122,14 +2122,6 @@ perform_pullup_replace_vars(PlannerInfo *root,
parse->returningList = (List *)
pullup_replace_vars((Node *) parse->returningList, rvcontext);
- foreach(lc, parse->windowClause)
- {
- WindowClause *wc = lfirst_node(WindowClause, lc);
-
- if (wc->runCondition != NIL)
- wc->runCondition = (List *)
- pullup_replace_vars((Node *) wc->runCondition, rvcontext);
- }
if (parse->onConflict)
{
parse->onConflict->onConflictSet = (List *)
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 94eb56a1e7..c9b4a0f5eb 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -2545,6 +2545,7 @@ eval_const_expressions_mutator(Node *node,
newexpr->inputcollid = expr->inputcollid;
newexpr->args = args;
newexpr->aggfilter = aggfilter;
+ newexpr->runCondition = expr->runCondition;
newexpr->winref = expr->winref;
newexpr->winstar = expr->winstar;
newexpr->winagg = expr->winagg;
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 8dbf790e89..1618be8c8e 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -3467,6 +3467,7 @@ create_windowagg_path(PlannerInfo *root,
Path *subpath,
PathTarget *target,
List *windowFuncs,
+ List *rundcondition,
WindowClause *winclause,
List *qual,
bool topwindow)
@@ -3491,6 +3492,7 @@ create_windowagg_path(PlannerInfo *root,
pathnode->subpath = subpath;
pathnode->winclause = winclause;
pathnode->qual = qual;
+ pathnode->runCondition = rundcondition;
pathnode->topwindow = topwindow;
/*
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index 4b50278fd0..219bf34c33 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -2950,7 +2950,6 @@ transformWindowDefinitions(ParseState *pstate,
rangeopfamily, rangeopcintype,
&wc->endInRangeFunc,
windef->endOffset);
- wc->runCondition = NIL;
wc->winref = winref;
result = lappend(result, wc);
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 9300c7b9ab..2b7d66bef3 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -3734,6 +3734,7 @@ transformJsonAggConstructor(ParseState *pstate, JsonAggConstructor *agg_ctor,
/* wincollid and inputcollid will be set by parse_collate.c */
wfunc->args = args;
wfunc->aggfilter = aggfilter;
+ wfunc->runCondition = NIL;
/* winref will be set by transformWindowFuncCall */
wfunc->winstar = false;
wfunc->winagg = true;
diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index fdb3e6df33..42109535bb 100644
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -834,6 +834,7 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
wfunc->winstar = agg_star;
wfunc->winagg = (fdresult == FUNCDETAIL_AGGREGATE);
wfunc->aggfilter = agg_filter;
+ wfunc->runCondition = NIL;
wfunc->location = location;
/*
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 476d55dd24..9eb53b44dd 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1492,8 +1492,6 @@ typedef struct WindowClause
int frameOptions; /* frame_clause options, see WindowDef */
Node *startOffset; /* expression for starting bound, if any */
Node *endOffset; /* expression for ending bound, if any */
- /* qual to help short-circuit execution */
- List *runCondition pg_node_attr(query_jumble_ignore);
/* in_range function for startOffset */
Oid startInRangeFunc pg_node_attr(query_jumble_ignore);
/* in_range function for endOffset */
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index 534692bee1..c8cebfe375 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -2299,6 +2299,7 @@ typedef struct WindowAggPath
Path *subpath; /* path representing input source */
WindowClause *winclause; /* WindowClause we'll be using */
List *qual; /* lower-level WindowAgg runconditions */
+ List *runCondition; /* List of WindowFuncRunConditions */
bool topwindow; /* false for all apart from the WindowAgg
* that's closest to the root of the plan */
} WindowAggPath;
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index 4a154606d2..c17d049bb5 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -561,6 +561,8 @@ typedef struct WindowFunc
List *args;
/* FILTER expression, if any */
Expr *aggfilter;
+ /* List of WindowFuncRunConditions to help short-circuit execution */
+ List *runCondition pg_node_attr(query_jumble_ignore);
/* index of associated WindowClause */
Index winref;
/* true if argument list was really '*' */
@@ -571,6 +573,21 @@ typedef struct WindowFunc
int location;
} WindowFunc;
+
+typedef struct WindowFuncRunCondition
+{
+ Expr xpr;
+
+ /* PG_OPERATOR OID of the operator */
+ Oid opno;
+ /* OID of collation that operator should use */
+ Oid inputcollid pg_node_attr(query_jumble_ignore);
+ /* true of WindowFunc belongs on the left of the resulting OpExpr */
+ bool wfunc_left;
+ /* argument to the run condition */
+ Expr *arg;
+} WindowFuncRunCondition;
+
/*
* SubscriptingRef: describes a subscripting operation over a container
* (array, etc).
diff --git a/src/include/optimizer/pathnode.h b/src/include/optimizer/pathnode.h
index c43d97b48a..38faf607b2 100644
--- a/src/include/optimizer/pathnode.h
+++ b/src/include/optimizer/pathnode.h
@@ -250,6 +250,7 @@ extern WindowAggPath *create_windowagg_path(PlannerInfo *root,
Path *subpath,
PathTarget *target,
List *windowFuncs,
+ List *runcondition,
WindowClause *winclause,
List *qual,
bool topwindow);
diff --git a/src/test/regress/expected/window.out b/src/test/regress/expected/window.out
index 60de2cbf96..95404e7432 100644
--- a/src/test/regress/expected/window.out
+++ b/src/test/regress/expected/window.out
@@ -4207,6 +4207,23 @@ WHERE s.c = 1;
-> Seq Scan on empsalary e2
(14 rows)
+-- A case where the WindowFunc has Vars from other query levels
+EXPLAIN (COSTS OFF)
+SELECT 1 FROM
+ (SELECT ntile(s1.x) OVER () AS c
+ FROM (SELECT (SELECT 1) AS x) AS s1) s
+WHERE s.c = 1;
+ QUERY PLAN
+--------------------------------------------------
+ Subquery Scan on s
+ Filter: (s.c = 1)
+ -> WindowAgg
+ Run Condition: (ntile($0) OVER (?) <= 1)
+ InitPlan 1 (returns $0)
+ -> Result
+ -> Result
+(7 rows)
+
-- Tests to ensure we don't push down the run condition when it's not valid to
-- do so.
-- Ensure we don't push down when the frame options show that the window
diff --git a/src/test/regress/sql/window.sql b/src/test/regress/sql/window.sql
index 678aa1a21c..7da89ebf5c 100644
--- a/src/test/regress/sql/window.sql
+++ b/src/test/regress/sql/window.sql
@@ -1377,6 +1377,13 @@ SELECT 1 FROM
WHERE e1.empno = e2.empno) s
WHERE s.c = 1;
+-- A case where the WindowFunc has Vars from other query levels
+EXPLAIN (COSTS OFF)
+SELECT 1 FROM
+ (SELECT ntile(s1.x) OVER () AS c
+ FROM (SELECT (SELECT 1) AS x) AS s1) s
+WHERE s.c = 1;
+
-- Tests to ensure we don't push down the run condition when it's not valid to
-- do so.
* Re: BUG #18305: Unexpected error: "WindowFunc not found in subplan target lists" triggered by subqueries
@ 2024-03-22 08:55 Richard Guo <guofenglinux@gmail.com>
parent: David Rowley <dgrowleyml@gmail.com>
1 sibling, 0 replies; 15+ messages in thread
From: Richard Guo @ 2024-03-22 08:55 UTC (permalink / raw)
To: David Rowley <dgrowleyml@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; zuming.jiang@inf.ethz.ch; pgsql-bugs@lists.postgresql.org
On Fri, Jan 26, 2024 at 8:02 PM David Rowley <dgrowleyml@gmail.com> wrote:
> On Thu, 25 Jan 2024 at 18:14, David Rowley <dgrowleyml@gmail.com> wrote:
> > If WindowFunc allowed a list of a new struct called WindowRunCondition
> > with fields "otherarg", "opno", "collation", "wfunc_left" then we
> > could construct the OpExpr later either in createplan.c or setrefs.c.
> > The EXPLAIN version of that OpExpr could have the WindowFunc and the
> > non-EXPLAIN version would have the Var.
>
> Just to assist the discussion here I've drafted a patch along the
> lines of the above. See attached
>
> If you think this idea has merit I can try and turn it into something
> committable for master.
This idea seems reasonable to me. Now the runCondition is constructed
in create_one_window_path(), where the subquery has been through
preprocessing and therefore the WindowFunc's arg has been replaced with
a Param due to the pullup of the lower subquery and the expansion of
SubLinks to SubPlans. This fixes the problem reported here.
Thanks
Richard
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #18305: Unexpected error: "WindowFunc not found in subplan target lists" triggered by subqueries
@ 2024-03-22 10:47 Richard Guo <guofenglinux@gmail.com>
parent: David Rowley <dgrowleyml@gmail.com>
1 sibling, 1 reply; 15+ messages in thread
From: Richard Guo @ 2024-03-22 10:47 UTC (permalink / raw)
To: David Rowley <dgrowleyml@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; zuming.jiang@inf.ethz.ch; pgsql-bugs@lists.postgresql.org
On Thu, Jan 25, 2024 at 1:14 PM David Rowley <dgrowleyml@gmail.com> wrote:
> Sounds a bit invasive for back branches, but wondering if we couldn't
> just modify window_ntile_support() to reject any ntile args other than
> Consts. count(*), row_number(), rank(), dense_rank(), percent_rank()
> and percent_rank() all can't suffer from this issue as they don't have
> an argument. count(expr) would need to have something done to stop
> the same issue from occurring. Maybe int8inc_support() could just set
> req->monotonic = MONOTONICFUNC_NONE if the req->window_func has an
> arg, effectively disabling the optimisation for count(expr).
You're right that count(expr) also suffers from this issue.
select 1 from
(select count(s1.x) over () as c
from (select (select 1) as x) as s1) s
where s.c = 1;
ERROR: WindowFunc not found in subplan target lists
For back branches, the idea of modifying window_ntile_support() and
int8inc_support() to reject any non-pseudoconstant args also seems
reasonable to me. One thing I noticed is that sometimes it's not easy
to tell whether the arg is pseudoconstant or not in the support
functions, because a pseudoconstant is not necessarily being type of
Const. For instance, count(1::text) is a CoerceViaIO, and
ntile(1.0::int) is a FuncExpr. But these are very corner cases and I
think we can just ignore them.
Thanks
Richard
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Re: BUG #18305: Unexpected error: "WindowFunc not found in subplan target lists" triggered by subqueries
@ 2024-04-15 03:42 David Rowley <dgrowleyml@gmail.com>
parent: David Rowley <dgrowleyml@gmail.com>
1 sibling, 0 replies; 15+ messages in thread
From: David Rowley @ 2024-04-15 03:42 UTC (permalink / raw)
To: akuluasan <akuluasan@163.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Richard Guo <guofenglinux@gmail.com>; zuming.jiang@inf.ethz.ch; pgsql-bugs@lists.postgresql.org
On Mon, 1 Apr 2024 at 21:34, akuluasan <akuluasan@163.com> wrote:
> I am using my tool to simplify the SQL query. Can you please confirm if the simplification process helps you diagnose and locate bugs?
The root cause of this has already been established, so in this case,
the answer is "no".
However, since you wrote "bugs" rather than "this bug", for cases
where we've not found the root cause, the more simple the reproducer,
the better.
David
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #18305: Unexpected error: "WindowFunc not found in subplan target lists" triggered by subqueries
@ 2024-04-24 02:05 David Rowley <dgrowleyml@gmail.com>
parent: Richard Guo <guofenglinux@gmail.com>
0 siblings, 1 reply; 15+ messages in thread
From: David Rowley @ 2024-04-24 02:05 UTC (permalink / raw)
To: Richard Guo <guofenglinux@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; zuming.jiang@inf.ethz.ch; pgsql-bugs@lists.postgresql.org
On Fri, 22 Mar 2024 at 23:47, Richard Guo <guofenglinux@gmail.com> wrote:
> For back branches, the idea of modifying window_ntile_support() and
> int8inc_support() to reject any non-pseudoconstant args also seems
> reasonable to me. One thing I noticed is that sometimes it's not easy
> to tell whether the arg is pseudoconstant or not in the support
> functions, because a pseudoconstant is not necessarily being type of
> Const. For instance, count(1::text) is a CoerceViaIO, and
> ntile(1.0::int) is a FuncExpr. But these are very corner cases and I
> think we can just ignore them.
I don't think that needs anything special aside from constant folding.
I've attached a more complete version of the patch (0002) and another
patch which is what I'd proposed as a fix for the backbranches (0001).
Note quite a few tests needed to be adjusted because of disabling this
optimisation.
The 0002 patch will require a cat version bump as it adds a field to
WindowFunc. Ideally, I'd be applying this fix to master, but I
imagine some people might feel we should delay applying a fix like
this until after we branch for v18. Happy to hear people's views on
that.
David
Attachments: download all (2 files, .tar.gz)
v2-0001-Disable-run-conditions-for-ntile-var-and-count-va.patch (text/plain, 12.3K) download
From 97812b07cd07bbd65b206f792093d8451ea4fe72 Mon Sep 17 00:00:00 2001
From: David Rowley <dgrowley@gmail.com>
Date: Tue, 23 Apr 2024 14:31:11 +1200
Subject: [PATCH v2 1/2] Disable run conditions for ntile(var) and count(var)
---
src/backend/utils/adt/int8.c | 9 +++
src/backend/utils/adt/windowfuncs.c | 24 +++++--
src/test/regress/expected/window.out | 94 +++++++++++++++-------------
src/test/regress/sql/window.sql | 43 ++++++++-----
4 files changed, 105 insertions(+), 65 deletions(-)
diff --git a/src/backend/utils/adt/int8.c b/src/backend/utils/adt/int8.c
index 54fa3bc379..e7b1d389b3 100644
--- a/src/backend/utils/adt/int8.c
+++ b/src/backend/utils/adt/int8.c
@@ -832,6 +832,15 @@ int8inc_support(PG_FUNCTION_ARGS)
SupportRequestWFuncMonotonic *req = (SupportRequestWFuncMonotonic *) rawreq;
MonotonicFunction monotonic = MONOTONICFUNC_NONE;
int frameOptions = req->window_clause->frameOptions;
+ WindowFunc *wfunc = req->window_func;
+
+ if (list_length(wfunc->args) == 1)
+ {
+ Node *expr = eval_const_expressions(NULL, linitial(wfunc->args));
+
+ if (!IsA(expr, Const))
+ PG_RETURN_POINTER(NULL);
+ }
/* No ORDER BY clause then all rows are peers */
if (req->window_clause->orderClause == NIL)
diff --git a/src/backend/utils/adt/windowfuncs.c b/src/backend/utils/adt/windowfuncs.c
index 473c61569f..b49e48109d 100644
--- a/src/backend/utils/adt/windowfuncs.c
+++ b/src/backend/utils/adt/windowfuncs.c
@@ -16,6 +16,7 @@
#include "nodes/parsenodes.h"
#include "nodes/supportnodes.h"
#include "utils/fmgrprotos.h"
+#include "optimizer/optimizer.h"
#include "windowapi.h"
/*
@@ -487,13 +488,24 @@ window_ntile_support(PG_FUNCTION_ARGS)
if (IsA(rawreq, SupportRequestWFuncMonotonic))
{
SupportRequestWFuncMonotonic *req = (SupportRequestWFuncMonotonic *) rawreq;
+ WindowFunc *wfunc = req->window_func;
- /*
- * ntile() is monotonically increasing as the number of buckets cannot
- * change after the first call
- */
- req->monotonic = MONOTONICFUNC_INCREASING;
- PG_RETURN_POINTER(req);
+ if (list_length(wfunc->args) == 1)
+ {
+ Node *expr = eval_const_expressions(NULL, linitial(wfunc->args));
+
+ if (IsA(expr, Const))
+ {
+ /*
+ * ntile() is monotonically increasing as the number of
+ * buckets cannot change after the first call
+ */
+ req->monotonic = MONOTONICFUNC_INCREASING;
+ PG_RETURN_POINTER(req);
+ }
+ }
+
+ PG_RETURN_POINTER(NULL);
}
if (IsA(rawreq, SupportRequestOptimizeWindowClause))
diff --git a/src/test/regress/expected/window.out b/src/test/regress/expected/window.out
index e46710cf31..2f2cb112b1 100644
--- a/src/test/regress/expected/window.out
+++ b/src/test/regress/expected/window.out
@@ -3954,13 +3954,13 @@ EXPLAIN (COSTS OFF)
SELECT * FROM
(SELECT empno,
salary,
- count(empno) OVER (ORDER BY salary DESC) c
+ count(1) OVER (ORDER BY salary DESC) c
FROM empsalary) emp
WHERE c <= 3;
- QUERY PLAN
----------------------------------------------------------
+ QUERY PLAN
+-------------------------------------------
WindowAgg
- Run Condition: (count(empsalary.empno) OVER (?) <= 3)
+ Run Condition: (count(1) OVER (?) <= 3)
-> Sort
Sort Key: empsalary.salary DESC
-> Seq Scan on empsalary
@@ -3969,7 +3969,7 @@ WHERE c <= 3;
SELECT * FROM
(SELECT empno,
salary,
- count(empno) OVER (ORDER BY salary DESC) c
+ count(1) OVER (ORDER BY salary DESC) c
FROM empsalary) emp
WHERE c <= 3;
empno | salary | c
@@ -4081,19 +4081,19 @@ WHERE rn < 3;
-> Seq Scan on empsalary
(6 rows)
--- likewise with count(empno) instead of row_number()
+-- likewise with count(1) instead of row_number()
EXPLAIN (COSTS OFF)
SELECT * FROM
(SELECT empno,
depname,
salary,
- count(empno) OVER (PARTITION BY depname ORDER BY salary DESC) c
+ count(1) OVER (PARTITION BY depname ORDER BY salary DESC) c
FROM empsalary) emp
WHERE c <= 3;
QUERY PLAN
------------------------------------------------------------
WindowAgg
- Run Condition: (count(empsalary.empno) OVER (?) <= 3)
+ Run Condition: (count(1) OVER (?) <= 3)
-> Sort
Sort Key: empsalary.depname, empsalary.salary DESC
-> Seq Scan on empsalary
@@ -4104,7 +4104,7 @@ SELECT * FROM
(SELECT empno,
depname,
salary,
- count(empno) OVER (PARTITION BY depname ORDER BY salary DESC) c
+ count(1) OVER (PARTITION BY depname ORDER BY salary DESC) c
FROM empsalary) emp
WHERE c <= 3;
empno | depname | salary | c
@@ -4126,13 +4126,13 @@ SELECT * FROM
(SELECT empno,
depname,
salary,
- count(empno) OVER () c
+ count(1) OVER () c
FROM empsalary) emp
WHERE c = 1;
- QUERY PLAN
---------------------------------------------------------
+ QUERY PLAN
+------------------------------------------
WindowAgg
- Run Condition: (count(empsalary.empno) OVER (?) = 1)
+ Run Condition: (count(1) OVER (?) = 1)
-> Seq Scan on empsalary
(3 rows)
@@ -4140,7 +4140,7 @@ WHERE c = 1;
EXPLAIN (COSTS OFF)
SELECT * FROM
(SELECT *,
- count(salary) OVER (PARTITION BY depname || '') c1, -- w1
+ count(1) OVER (PARTITION BY depname || '') c1, -- w1
row_number() OVER (PARTITION BY depname) rn, -- w2
count(*) OVER (PARTITION BY depname) c2, -- w2
count(*) OVER (PARTITION BY '' || depname) c3, -- w3
@@ -4152,7 +4152,7 @@ SELECT * FROM
Subquery Scan on e
-> WindowAgg
Filter: (((row_number() OVER (?)) <= 1) AND ((ntile(2) OVER (?)) < 2))
- Run Condition: (count(empsalary.salary) OVER (?) <= 3)
+ Run Condition: (count(1) OVER (?) <= 3)
-> Sort
Sort Key: (((empsalary.depname)::text || ''::text))
-> WindowAgg
@@ -4168,7 +4168,7 @@ SELECT * FROM
-- Ensure we correctly filter out all of the run conditions from each window
SELECT * FROM
(SELECT *,
- count(salary) OVER (PARTITION BY depname || '') c1, -- w1
+ count(1) OVER (PARTITION BY depname || '') c1, -- w1
row_number() OVER (PARTITION BY depname) rn, -- w2
count(*) OVER (PARTITION BY depname) c2, -- w2
count(*) OVER (PARTITION BY '' || depname) c3, -- w3
@@ -4181,32 +4181,6 @@ SELECT * FROM
sales | 3 | 4800 | 08-01-2007 | 3 | 1 | 3 | 3 | 1
(2 rows)
--- Ensure we remove references to reduced outer joins as nulling rels in run
--- conditions
-EXPLAIN (COSTS OFF)
-SELECT 1 FROM
- (SELECT ntile(e2.salary) OVER (PARTITION BY e1.depname) AS c
- FROM empsalary e1 LEFT JOIN empsalary e2 ON TRUE
- WHERE e1.empno = e2.empno) s
-WHERE s.c = 1;
- QUERY PLAN
----------------------------------------------------------
- Subquery Scan on s
- Filter: (s.c = 1)
- -> WindowAgg
- Run Condition: (ntile(e2.salary) OVER (?) <= 1)
- -> Sort
- Sort Key: e1.depname
- -> Merge Join
- Merge Cond: (e1.empno = e2.empno)
- -> Sort
- Sort Key: e1.empno
- -> Seq Scan on empsalary e1
- -> Sort
- Sort Key: e2.empno
- -> Seq Scan on empsalary e2
-(14 rows)
-
-- Tests to ensure we don't push down the run condition when it's not valid to
-- do so.
-- Ensure we don't push down when the frame options show that the window
@@ -4266,6 +4240,42 @@ WHERE c = 1;
-> Seq Scan on empsalary
(6 rows)
+-- Ensure we don't use a run condition when the WindowFunc arg contains a Var
+EXPLAIN (COSTS OFF)
+SELECT * FROM
+ (SELECT empno,
+ salary,
+ count(empno) OVER (ORDER BY empno DESC) c
+ FROM empsalary) emp
+WHERE c = 1;
+ QUERY PLAN
+----------------------------------------------
+ Subquery Scan on emp
+ Filter: (emp.c = 1)
+ -> WindowAgg
+ -> Sort
+ Sort Key: empsalary.empno DESC
+ -> Seq Scan on empsalary
+(6 rows)
+
+-- As above but with ntile().
+EXPLAIN (COSTS OFF)
+SELECT * FROM
+ (SELECT empno,
+ salary,
+ ntile(empno::int) OVER (ORDER BY empno DESC) nt
+ FROM empsalary) emp
+WHERE nt = 1;
+ QUERY PLAN
+----------------------------------------------
+ Subquery Scan on emp
+ Filter: (emp.nt = 1)
+ -> WindowAgg
+ -> Sort
+ Sort Key: empsalary.empno DESC
+ -> Seq Scan on empsalary
+(6 rows)
+
-- Ensure we don't use a run condition when the WindowFunc contains subplans
EXPLAIN (COSTS OFF)
SELECT * FROM
diff --git a/src/test/regress/sql/window.sql b/src/test/regress/sql/window.sql
index 678aa1a21c..284f47b514 100644
--- a/src/test/regress/sql/window.sql
+++ b/src/test/regress/sql/window.sql
@@ -1252,14 +1252,14 @@ EXPLAIN (COSTS OFF)
SELECT * FROM
(SELECT empno,
salary,
- count(empno) OVER (ORDER BY salary DESC) c
+ count(1) OVER (ORDER BY salary DESC) c
FROM empsalary) emp
WHERE c <= 3;
SELECT * FROM
(SELECT empno,
salary,
- count(empno) OVER (ORDER BY salary DESC) c
+ count(1) OVER (ORDER BY salary DESC) c
FROM empsalary) emp
WHERE c <= 3;
@@ -1315,13 +1315,13 @@ SELECT empno, depname FROM
FROM empsalary) emp
WHERE rn < 3;
--- likewise with count(empno) instead of row_number()
+-- likewise with count(1) instead of row_number()
EXPLAIN (COSTS OFF)
SELECT * FROM
(SELECT empno,
depname,
salary,
- count(empno) OVER (PARTITION BY depname ORDER BY salary DESC) c
+ count(1) OVER (PARTITION BY depname ORDER BY salary DESC) c
FROM empsalary) emp
WHERE c <= 3;
@@ -1330,7 +1330,7 @@ SELECT * FROM
(SELECT empno,
depname,
salary,
- count(empno) OVER (PARTITION BY depname ORDER BY salary DESC) c
+ count(1) OVER (PARTITION BY depname ORDER BY salary DESC) c
FROM empsalary) emp
WHERE c <= 3;
@@ -1341,7 +1341,7 @@ SELECT * FROM
(SELECT empno,
depname,
salary,
- count(empno) OVER () c
+ count(1) OVER () c
FROM empsalary) emp
WHERE c = 1;
@@ -1349,7 +1349,7 @@ WHERE c = 1;
EXPLAIN (COSTS OFF)
SELECT * FROM
(SELECT *,
- count(salary) OVER (PARTITION BY depname || '') c1, -- w1
+ count(1) OVER (PARTITION BY depname || '') c1, -- w1
row_number() OVER (PARTITION BY depname) rn, -- w2
count(*) OVER (PARTITION BY depname) c2, -- w2
count(*) OVER (PARTITION BY '' || depname) c3, -- w3
@@ -1360,7 +1360,7 @@ SELECT * FROM
-- Ensure we correctly filter out all of the run conditions from each window
SELECT * FROM
(SELECT *,
- count(salary) OVER (PARTITION BY depname || '') c1, -- w1
+ count(1) OVER (PARTITION BY depname || '') c1, -- w1
row_number() OVER (PARTITION BY depname) rn, -- w2
count(*) OVER (PARTITION BY depname) c2, -- w2
count(*) OVER (PARTITION BY '' || depname) c3, -- w3
@@ -1368,15 +1368,6 @@ SELECT * FROM
FROM empsalary
) e WHERE rn <= 1 AND c1 <= 3 AND nt < 2;
--- Ensure we remove references to reduced outer joins as nulling rels in run
--- conditions
-EXPLAIN (COSTS OFF)
-SELECT 1 FROM
- (SELECT ntile(e2.salary) OVER (PARTITION BY e1.depname) AS c
- FROM empsalary e1 LEFT JOIN empsalary e2 ON TRUE
- WHERE e1.empno = e2.empno) s
-WHERE s.c = 1;
-
-- Tests to ensure we don't push down the run condition when it's not valid to
-- do so.
@@ -1410,6 +1401,24 @@ SELECT * FROM
FROM empsalary) emp
WHERE c = 1;
+-- Ensure we don't use a run condition when the WindowFunc arg contains a Var
+EXPLAIN (COSTS OFF)
+SELECT * FROM
+ (SELECT empno,
+ salary,
+ count(empno) OVER (ORDER BY empno DESC) c
+ FROM empsalary) emp
+WHERE c = 1;
+
+-- As above but with ntile().
+EXPLAIN (COSTS OFF)
+SELECT * FROM
+ (SELECT empno,
+ salary,
+ ntile(empno::int) OVER (ORDER BY empno DESC) nt
+ FROM empsalary) emp
+WHERE nt = 1;
+
-- Ensure we don't use a run condition when the WindowFunc contains subplans
EXPLAIN (COSTS OFF)
SELECT * FROM
--
2.40.1
v2-0002-Re-instate-ntile-var-and-count-var-run-condition-.patch (text/plain, 29.7K) download
From 761e6f3f2fe0ee6dff68adf0abd22c58592ac57f Mon Sep 17 00:00:00 2001
From: David Rowley <dgrowley@gmail.com>
Date: Wed, 24 Apr 2024 12:49:08 +1200
Subject: [PATCH v2 2/2] Re-instate ntile(var) and count(var) run condition
optimization
Remember cat version bump
---
src/backend/nodes/nodeFuncs.c | 26 +++--
src/backend/optimizer/path/allpaths.c | 36 +++----
src/backend/optimizer/plan/createplan.c | 2 +-
src/backend/optimizer/plan/planner.c | 54 +++++++++--
src/backend/optimizer/prep/prepjointree.c | 8 --
src/backend/optimizer/util/clauses.c | 1 +
src/backend/optimizer/util/pathnode.c | 3 +
src/backend/parser/parse_clause.c | 1 -
src/backend/parser/parse_expr.c | 1 +
src/backend/parser/parse_func.c | 1 +
src/backend/utils/adt/int8.c | 9 --
src/backend/utils/adt/windowfuncs.c | 24 ++---
src/include/nodes/parsenodes.h | 2 -
src/include/nodes/pathnodes.h | 1 +
src/include/nodes/primnodes.h | 30 ++++++
src/include/optimizer/pathnode.h | 1 +
src/test/regress/expected/window.out | 112 ++++++++++++----------
src/test/regress/sql/window.sql | 51 +++++-----
src/tools/pgindent/typedefs.list | 1 +
19 files changed, 207 insertions(+), 157 deletions(-)
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index e1df1894b6..3db4a42dd7 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -2163,6 +2163,16 @@ expression_tree_walker_impl(Node *node,
return true;
if (WALK(expr->aggfilter))
return true;
+ if (WALK(expr->runCondition))
+ return true;
+ }
+ break;
+ case T_WindowFuncRunCondition:
+ {
+ WindowFuncRunCondition *expr = (WindowFuncRunCondition *) node;
+
+ if (WALK(expr->arg))
+ return true;
}
break;
case T_SubscriptingRef:
@@ -2400,8 +2410,6 @@ expression_tree_walker_impl(Node *node,
return true;
if (WALK(wc->endOffset))
return true;
- if (WALK(wc->runCondition))
- return true;
}
break;
case T_CTECycleClause:
@@ -2752,8 +2760,6 @@ query_tree_walker_impl(Query *query,
return true;
if (WALK(wc->endOffset))
return true;
- if (WALK(wc->runCondition))
- return true;
}
}
@@ -3053,6 +3059,16 @@ expression_tree_mutator_impl(Node *node,
return (Node *) newnode;
}
break;
+ case T_WindowFuncRunCondition:
+ {
+ WindowFuncRunCondition *expr = (WindowFuncRunCondition *) node;
+ WindowFuncRunCondition *newnode;
+
+ FLATCOPY(newnode, expr, WindowFuncRunCondition);
+ MUTATE(newnode->arg, expr->arg, Expr *);
+ return (Node *) newnode;
+ }
+ break;
case T_SubscriptingRef:
{
SubscriptingRef *sbsref = (SubscriptingRef *) node;
@@ -3466,7 +3482,6 @@ expression_tree_mutator_impl(Node *node,
MUTATE(newnode->orderClause, wc->orderClause, List *);
MUTATE(newnode->startOffset, wc->startOffset, Node *);
MUTATE(newnode->endOffset, wc->endOffset, Node *);
- MUTATE(newnode->runCondition, wc->runCondition, List *);
return (Node *) newnode;
}
break;
@@ -3799,7 +3814,6 @@ query_tree_mutator_impl(Query *query,
FLATCOPY(newnode, wc, WindowClause);
MUTATE(newnode->startOffset, wc->startOffset, Node *);
MUTATE(newnode->endOffset, wc->endOffset, Node *);
- MUTATE(newnode->runCondition, wc->runCondition, List *);
resultlist = lappend(resultlist, (Node *) newnode);
}
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index cc51ae1757..f21ae3ecda 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -2205,7 +2205,7 @@ set_dummy_rel_pathlist(RelOptInfo *rel)
* the run condition will handle all of the required filtering.
*
* Returns true if 'opexpr' was found to be useful and was added to the
- * WindowClauses runCondition. We also set *keep_original accordingly and add
+ * WindowFunc's runCondition. We also set *keep_original accordingly and add
* 'attno' to *run_cond_attrs offset by FirstLowInvalidHeapAttributeNumber.
* If the 'opexpr' cannot be used then we set *keep_original to true and
* return false.
@@ -2358,7 +2358,7 @@ find_window_run_conditions(Query *subquery, RangeTblEntry *rte, Index rti,
*keep_original = true;
runopexpr = opexpr;
- /* determine the operator to use for the runCondition qual */
+ /* determine the operator to use for the WindowFuncRunCondition */
runoperator = get_opfamily_member(opinfo->opfamily_id,
opinfo->oplefttype,
opinfo->oprighttype,
@@ -2369,27 +2369,15 @@ find_window_run_conditions(Query *subquery, RangeTblEntry *rte, Index rti,
if (runopexpr != NULL)
{
- Expr *newexpr;
+ WindowFuncRunCondition *runcond;
- /*
- * Build the qual required for the run condition keeping the
- * WindowFunc on the same side as it was originally.
- */
- if (wfunc_left)
- newexpr = make_opclause(runoperator,
- runopexpr->opresulttype,
- runopexpr->opretset, (Expr *) wfunc,
- otherexpr, runopexpr->opcollid,
- runopexpr->inputcollid);
- else
- newexpr = make_opclause(runoperator,
- runopexpr->opresulttype,
- runopexpr->opretset,
- otherexpr, (Expr *) wfunc,
- runopexpr->opcollid,
- runopexpr->inputcollid);
+ runcond = makeNode(WindowFuncRunCondition);
+ runcond->opno = runoperator;
+ runcond->inputcollid = runopexpr->inputcollid;
+ runcond->wfunc_left = wfunc_left;
+ runcond->arg = copyObject(otherexpr);
- wclause->runCondition = lappend(wclause->runCondition, newexpr);
+ wfunc->runCondition = lappend(wfunc->runCondition, runcond);
/* record that this attno was used in a run condition */
*run_cond_attrs = bms_add_member(*run_cond_attrs,
@@ -2403,9 +2391,9 @@ find_window_run_conditions(Query *subquery, RangeTblEntry *rte, Index rti,
/*
* check_and_push_window_quals
- * Check if 'clause' is a qual that can be pushed into a WindowFunc's
- * WindowClause as a 'runCondition' qual. These, when present, allow
- * some unnecessary work to be skipped during execution.
+ * Check if 'clause' is a qual that can be pushed into a WindowFunc
+ * as a 'runCondition' qual. These, when present, allow some unnecessary
+ * work to be skipped during execution.
*
* 'run_cond_attrs' will be populated with all targetlist resnos of subquery
* targets (offset by FirstLowInvalidHeapAttributeNumber) that we pushed
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 3b77886567..6b64c4a362 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -2699,7 +2699,7 @@ create_windowagg_plan(PlannerInfo *root, WindowAggPath *best_path)
wc->inRangeColl,
wc->inRangeAsc,
wc->inRangeNullsFirst,
- wc->runCondition,
+ best_path->runCondition,
best_path->qual,
best_path->topwindow,
subplan);
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 5320da51a0..b149b5690d 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -870,9 +870,6 @@ subquery_planner(PlannerGlobal *glob, Query *parse, PlannerInfo *parent_root,
EXPRKIND_LIMIT);
wc->endOffset = preprocess_expression(root, wc->endOffset,
EXPRKIND_LIMIT);
- wc->runCondition = (List *) preprocess_expression(root,
- (Node *) wc->runCondition,
- EXPRKIND_TARGET);
}
parse->limitOffset = preprocess_expression(root, parse->limitOffset,
@@ -4527,9 +4524,11 @@ create_one_window_path(PlannerInfo *root,
{
WindowClause *wc = lfirst_node(WindowClause, l);
List *window_pathkeys;
+ List *runcondition = NIL;
int presorted_keys;
bool is_sorted;
bool topwindow;
+ ListCell *lc2;
window_pathkeys = make_pathkeys_for_window(root,
wc,
@@ -4577,7 +4576,6 @@ create_one_window_path(PlannerInfo *root,
* we do need to account for the increase in tlist width.
*/
int64 tuple_width = window_target->width;
- ListCell *lc2;
window_target = copy_pathtarget(window_target);
foreach(lc2, wflists->windowFuncs[wc->winref])
@@ -4599,17 +4597,53 @@ create_one_window_path(PlannerInfo *root,
topwindow = foreach_current_index(l) == list_length(activeWindows) - 1;
/*
- * Accumulate all of the runConditions from each intermediate
- * WindowClause. The top-level WindowAgg must pass these as a qual so
- * that it filters out unwanted tuples correctly.
+ * Collect the WindowFuncRunConditions from each WindowFunc and
+ * convert them into OpExprs
*/
- if (!topwindow)
- topqual = list_concat(topqual, wc->runCondition);
+ foreach(lc2, wflists->windowFuncs[wc->winref])
+ {
+ ListCell *lc3;
+ WindowFunc *wfunc = lfirst_node(WindowFunc, lc2);
+
+ foreach(lc3, wfunc->runCondition)
+ {
+ WindowFuncRunCondition *rc =
+ lfirst_node(WindowFuncRunCondition, lc3);
+ Expr *opexpr;
+ Expr *leftop;
+ Expr *rightop;
+
+ if (rc->wfunc_left)
+ {
+ leftop = (Expr *) copyObject(wfunc);
+ rightop = copyObject(rc->arg);
+ }
+ else
+ {
+ leftop = copyObject(rc->arg);
+ rightop = (Expr *) copyObject(wfunc);
+ }
+
+ opexpr = make_opclause(rc->opno,
+ BOOLOID,
+ false,
+ leftop,
+ rightop,
+ InvalidOid,
+ rc->inputcollid);
+
+ runcondition = lappend(runcondition, opexpr);
+
+ if (!topwindow)
+ topqual = lappend(topqual, opexpr);
+ }
+ }
path = (Path *)
create_windowagg_path(root, window_rel, path, window_target,
wflists->windowFuncs[wc->winref],
- wc, topwindow ? topqual : NIL, topwindow);
+ runcondition, wc,
+ topwindow ? topqual : NIL, topwindow);
}
add_path(window_rel, path);
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index 41da670f15..5482ab85a7 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -2175,14 +2175,6 @@ perform_pullup_replace_vars(PlannerInfo *root,
parse->returningList = (List *)
pullup_replace_vars((Node *) parse->returningList, rvcontext);
- foreach(lc, parse->windowClause)
- {
- WindowClause *wc = lfirst_node(WindowClause, lc);
-
- if (wc->runCondition != NIL)
- wc->runCondition = (List *)
- pullup_replace_vars((Node *) wc->runCondition, rvcontext);
- }
if (parse->onConflict)
{
parse->onConflict->onConflictSet = (List *)
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 59487cbd79..b4e085e9d4 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -2566,6 +2566,7 @@ eval_const_expressions_mutator(Node *node,
newexpr->inputcollid = expr->inputcollid;
newexpr->args = args;
newexpr->aggfilter = aggfilter;
+ newexpr->runCondition = expr->runCondition;
newexpr->winref = expr->winref;
newexpr->winstar = expr->winstar;
newexpr->winagg = expr->winagg;
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 3cf1dac087..3491c3af1c 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -3471,6 +3471,7 @@ create_minmaxagg_path(PlannerInfo *root,
* 'subpath' is the path representing the source of data
* 'target' is the PathTarget to be computed
* 'windowFuncs' is a list of WindowFunc structs
+ * 'runCondition' is a list of OpExprs to short-circuit WindowAgg execution
* 'winclause' is a WindowClause that is common to all the WindowFuncs
* 'qual' WindowClause.runconditions from lower-level WindowAggPaths.
* Must always be NIL when topwindow == false
@@ -3486,6 +3487,7 @@ create_windowagg_path(PlannerInfo *root,
Path *subpath,
PathTarget *target,
List *windowFuncs,
+ List *runCondition,
WindowClause *winclause,
List *qual,
bool topwindow)
@@ -3510,6 +3512,7 @@ create_windowagg_path(PlannerInfo *root,
pathnode->subpath = subpath;
pathnode->winclause = winclause;
pathnode->qual = qual;
+ pathnode->runCondition = runCondition;
pathnode->topwindow = topwindow;
/*
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index 4fc5fc87e0..8118036495 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -2956,7 +2956,6 @@ transformWindowDefinitions(ParseState *pstate,
rangeopfamily, rangeopcintype,
&wc->endInRangeFunc,
windef->endOffset);
- wc->runCondition = NIL;
wc->winref = winref;
result = lappend(result, wc);
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 1c1c86aa3e..aba3546ed1 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -3826,6 +3826,7 @@ transformJsonAggConstructor(ParseState *pstate, JsonAggConstructor *agg_ctor,
/* wincollid and inputcollid will be set by parse_collate.c */
wfunc->args = args;
wfunc->aggfilter = aggfilter;
+ wfunc->runCondition = NIL;
/* winref will be set by transformWindowFuncCall */
wfunc->winstar = false;
wfunc->winagg = true;
diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index 0cbc950c95..9b23344a3b 100644
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -834,6 +834,7 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
wfunc->winstar = agg_star;
wfunc->winagg = (fdresult == FUNCDETAIL_AGGREGATE);
wfunc->aggfilter = agg_filter;
+ wfunc->runCondition = NIL;
wfunc->location = location;
/*
diff --git a/src/backend/utils/adt/int8.c b/src/backend/utils/adt/int8.c
index e7b1d389b3..54fa3bc379 100644
--- a/src/backend/utils/adt/int8.c
+++ b/src/backend/utils/adt/int8.c
@@ -832,15 +832,6 @@ int8inc_support(PG_FUNCTION_ARGS)
SupportRequestWFuncMonotonic *req = (SupportRequestWFuncMonotonic *) rawreq;
MonotonicFunction monotonic = MONOTONICFUNC_NONE;
int frameOptions = req->window_clause->frameOptions;
- WindowFunc *wfunc = req->window_func;
-
- if (list_length(wfunc->args) == 1)
- {
- Node *expr = eval_const_expressions(NULL, linitial(wfunc->args));
-
- if (!IsA(expr, Const))
- PG_RETURN_POINTER(NULL);
- }
/* No ORDER BY clause then all rows are peers */
if (req->window_clause->orderClause == NIL)
diff --git a/src/backend/utils/adt/windowfuncs.c b/src/backend/utils/adt/windowfuncs.c
index b49e48109d..473c61569f 100644
--- a/src/backend/utils/adt/windowfuncs.c
+++ b/src/backend/utils/adt/windowfuncs.c
@@ -16,7 +16,6 @@
#include "nodes/parsenodes.h"
#include "nodes/supportnodes.h"
#include "utils/fmgrprotos.h"
-#include "optimizer/optimizer.h"
#include "windowapi.h"
/*
@@ -488,24 +487,13 @@ window_ntile_support(PG_FUNCTION_ARGS)
if (IsA(rawreq, SupportRequestWFuncMonotonic))
{
SupportRequestWFuncMonotonic *req = (SupportRequestWFuncMonotonic *) rawreq;
- WindowFunc *wfunc = req->window_func;
- if (list_length(wfunc->args) == 1)
- {
- Node *expr = eval_const_expressions(NULL, linitial(wfunc->args));
-
- if (IsA(expr, Const))
- {
- /*
- * ntile() is monotonically increasing as the number of
- * buckets cannot change after the first call
- */
- req->monotonic = MONOTONICFUNC_INCREASING;
- PG_RETURN_POINTER(req);
- }
- }
-
- PG_RETURN_POINTER(NULL);
+ /*
+ * ntile() is monotonically increasing as the number of buckets cannot
+ * change after the first call
+ */
+ req->monotonic = MONOTONICFUNC_INCREASING;
+ PG_RETURN_POINTER(req);
}
if (IsA(rawreq, SupportRequestOptimizeWindowClause))
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index c5f34efe27..28d969f47a 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1549,8 +1549,6 @@ typedef struct WindowClause
int frameOptions; /* frame_clause options, see WindowDef */
Node *startOffset; /* expression for starting bound, if any */
Node *endOffset; /* expression for ending bound, if any */
- /* qual to help short-circuit execution */
- List *runCondition pg_node_attr(query_jumble_ignore);
/* in_range function for startOffset */
Oid startInRangeFunc pg_node_attr(query_jumble_ignore);
/* in_range function for endOffset */
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index b8141f141a..99bd371e0a 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -2308,6 +2308,7 @@ typedef struct WindowAggPath
Path *subpath; /* path representing input source */
WindowClause *winclause; /* WindowClause we'll be using */
List *qual; /* lower-level WindowAgg runconditions */
+ List *runCondition; /* OpExpr List to short-circuit execution */
bool topwindow; /* false for all apart from the WindowAgg
* that's closest to the root of the plan */
} WindowAggPath;
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index 247cecb4b4..1ea5b65f53 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -575,6 +575,8 @@ typedef struct WindowFunc
List *args;
/* FILTER expression, if any */
Expr *aggfilter;
+ /* List of WindowFuncRunConditions to help short-circuit execution */
+ List *runCondition pg_node_attr(query_jumble_ignore);
/* index of associated WindowClause */
Index winref;
/* true if argument list was really '*' */
@@ -585,6 +587,34 @@ typedef struct WindowFunc
ParseLoc location;
} WindowFunc;
+/*
+ * WindowFuncRunCondition
+ *
+ * Represents intermediate OpExprs which will be used by WindowAgg to
+ * short-circuit execution.
+ */
+typedef struct WindowFuncRunCondition
+{
+ Expr xpr;
+
+ /* PG_OPERATOR OID of the operator */
+ Oid opno;
+ /* OID of collation that operator should use */
+ Oid inputcollid pg_node_attr(query_jumble_ignore);
+
+ /*
+ * true of WindowFunc belongs on the left of the resulting OpExpr or false
+ * if the WindowFunc is on the right.
+ */
+ bool wfunc_left;
+
+ /*
+ * The Expr being compared to the WindowFunc to use in the OpExpr in the
+ * WindowAgg's runCondition
+ */
+ Expr *arg;
+} WindowFuncRunCondition;
+
/*
* MergeSupportFunc
*
diff --git a/src/include/optimizer/pathnode.h b/src/include/optimizer/pathnode.h
index c5c4756b0f..112e7c23d4 100644
--- a/src/include/optimizer/pathnode.h
+++ b/src/include/optimizer/pathnode.h
@@ -250,6 +250,7 @@ extern WindowAggPath *create_windowagg_path(PlannerInfo *root,
Path *subpath,
PathTarget *target,
List *windowFuncs,
+ List *runCondition,
WindowClause *winclause,
List *qual,
bool topwindow);
diff --git a/src/test/regress/expected/window.out b/src/test/regress/expected/window.out
index 2f2cb112b1..ae4e8851f8 100644
--- a/src/test/regress/expected/window.out
+++ b/src/test/regress/expected/window.out
@@ -3954,13 +3954,13 @@ EXPLAIN (COSTS OFF)
SELECT * FROM
(SELECT empno,
salary,
- count(1) OVER (ORDER BY salary DESC) c
+ count(empno) OVER (ORDER BY salary DESC) c
FROM empsalary) emp
WHERE c <= 3;
- QUERY PLAN
--------------------------------------------
+ QUERY PLAN
+---------------------------------------------------------
WindowAgg
- Run Condition: (count(1) OVER (?) <= 3)
+ Run Condition: (count(empsalary.empno) OVER (?) <= 3)
-> Sort
Sort Key: empsalary.salary DESC
-> Seq Scan on empsalary
@@ -3969,7 +3969,7 @@ WHERE c <= 3;
SELECT * FROM
(SELECT empno,
salary,
- count(1) OVER (ORDER BY salary DESC) c
+ count(empno) OVER (ORDER BY salary DESC) c
FROM empsalary) emp
WHERE c <= 3;
empno | salary | c
@@ -4081,19 +4081,19 @@ WHERE rn < 3;
-> Seq Scan on empsalary
(6 rows)
--- likewise with count(1) instead of row_number()
+-- likewise with count(empno) instead of row_number()
EXPLAIN (COSTS OFF)
SELECT * FROM
(SELECT empno,
depname,
salary,
- count(1) OVER (PARTITION BY depname ORDER BY salary DESC) c
+ count(empno) OVER (PARTITION BY depname ORDER BY salary DESC) c
FROM empsalary) emp
WHERE c <= 3;
QUERY PLAN
------------------------------------------------------------
WindowAgg
- Run Condition: (count(1) OVER (?) <= 3)
+ Run Condition: (count(empsalary.empno) OVER (?) <= 3)
-> Sort
Sort Key: empsalary.depname, empsalary.salary DESC
-> Seq Scan on empsalary
@@ -4104,7 +4104,7 @@ SELECT * FROM
(SELECT empno,
depname,
salary,
- count(1) OVER (PARTITION BY depname ORDER BY salary DESC) c
+ count(empno) OVER (PARTITION BY depname ORDER BY salary DESC) c
FROM empsalary) emp
WHERE c <= 3;
empno | depname | salary | c
@@ -4126,13 +4126,13 @@ SELECT * FROM
(SELECT empno,
depname,
salary,
- count(1) OVER () c
+ count(empno) OVER () c
FROM empsalary) emp
WHERE c = 1;
- QUERY PLAN
-------------------------------------------
+ QUERY PLAN
+--------------------------------------------------------
WindowAgg
- Run Condition: (count(1) OVER (?) = 1)
+ Run Condition: (count(empsalary.empno) OVER (?) = 1)
-> Seq Scan on empsalary
(3 rows)
@@ -4140,7 +4140,7 @@ WHERE c = 1;
EXPLAIN (COSTS OFF)
SELECT * FROM
(SELECT *,
- count(1) OVER (PARTITION BY depname || '') c1, -- w1
+ count(salary) OVER (PARTITION BY depname || '') c1, -- w1
row_number() OVER (PARTITION BY depname) rn, -- w2
count(*) OVER (PARTITION BY depname) c2, -- w2
count(*) OVER (PARTITION BY '' || depname) c3, -- w3
@@ -4152,7 +4152,7 @@ SELECT * FROM
Subquery Scan on e
-> WindowAgg
Filter: (((row_number() OVER (?)) <= 1) AND ((ntile(2) OVER (?)) < 2))
- Run Condition: (count(1) OVER (?) <= 3)
+ Run Condition: (count(empsalary.salary) OVER (?) <= 3)
-> Sort
Sort Key: (((empsalary.depname)::text || ''::text))
-> WindowAgg
@@ -4168,7 +4168,7 @@ SELECT * FROM
-- Ensure we correctly filter out all of the run conditions from each window
SELECT * FROM
(SELECT *,
- count(1) OVER (PARTITION BY depname || '') c1, -- w1
+ count(salary) OVER (PARTITION BY depname || '') c1, -- w1
row_number() OVER (PARTITION BY depname) rn, -- w2
count(*) OVER (PARTITION BY depname) c2, -- w2
count(*) OVER (PARTITION BY '' || depname) c3, -- w3
@@ -4181,6 +4181,50 @@ SELECT * FROM
sales | 3 | 4800 | 08-01-2007 | 3 | 1 | 3 | 3 | 1
(2 rows)
+-- Ensure we remove references to reduced outer joins as nulling rels in run
+-- conditions
+EXPLAIN (COSTS OFF)
+SELECT 1 FROM
+ (SELECT ntile(e2.salary) OVER (PARTITION BY e1.depname) AS c
+ FROM empsalary e1 LEFT JOIN empsalary e2 ON TRUE
+ WHERE e1.empno = e2.empno) s
+WHERE s.c = 1;
+ QUERY PLAN
+---------------------------------------------------------
+ Subquery Scan on s
+ Filter: (s.c = 1)
+ -> WindowAgg
+ Run Condition: (ntile(e2.salary) OVER (?) <= 1)
+ -> Sort
+ Sort Key: e1.depname
+ -> Merge Join
+ Merge Cond: (e1.empno = e2.empno)
+ -> Sort
+ Sort Key: e1.empno
+ -> Seq Scan on empsalary e1
+ -> Sort
+ Sort Key: e2.empno
+ -> Seq Scan on empsalary e2
+(14 rows)
+
+-- Ensure the run condition optimization is used in cases where the WindowFunc
+-- has a Var from another query level
+EXPLAIN (COSTS OFF)
+SELECT 1 FROM
+ (SELECT ntile(s1.x) OVER () AS c
+ FROM (SELECT (SELECT 1) AS x) AS s1) s
+WHERE s.c = 1;
+ QUERY PLAN
+-----------------------------------------------------------------
+ Subquery Scan on s
+ Filter: (s.c = 1)
+ -> WindowAgg
+ Run Condition: (ntile((InitPlan 1).col1) OVER (?) <= 1)
+ InitPlan 1
+ -> Result
+ -> Result
+(7 rows)
+
-- Tests to ensure we don't push down the run condition when it's not valid to
-- do so.
-- Ensure we don't push down when the frame options show that the window
@@ -4240,42 +4284,6 @@ WHERE c = 1;
-> Seq Scan on empsalary
(6 rows)
--- Ensure we don't use a run condition when the WindowFunc arg contains a Var
-EXPLAIN (COSTS OFF)
-SELECT * FROM
- (SELECT empno,
- salary,
- count(empno) OVER (ORDER BY empno DESC) c
- FROM empsalary) emp
-WHERE c = 1;
- QUERY PLAN
-----------------------------------------------
- Subquery Scan on emp
- Filter: (emp.c = 1)
- -> WindowAgg
- -> Sort
- Sort Key: empsalary.empno DESC
- -> Seq Scan on empsalary
-(6 rows)
-
--- As above but with ntile().
-EXPLAIN (COSTS OFF)
-SELECT * FROM
- (SELECT empno,
- salary,
- ntile(empno::int) OVER (ORDER BY empno DESC) nt
- FROM empsalary) emp
-WHERE nt = 1;
- QUERY PLAN
-----------------------------------------------
- Subquery Scan on emp
- Filter: (emp.nt = 1)
- -> WindowAgg
- -> Sort
- Sort Key: empsalary.empno DESC
- -> Seq Scan on empsalary
-(6 rows)
-
-- Ensure we don't use a run condition when the WindowFunc contains subplans
EXPLAIN (COSTS OFF)
SELECT * FROM
diff --git a/src/test/regress/sql/window.sql b/src/test/regress/sql/window.sql
index 284f47b514..6de5493b05 100644
--- a/src/test/regress/sql/window.sql
+++ b/src/test/regress/sql/window.sql
@@ -1252,14 +1252,14 @@ EXPLAIN (COSTS OFF)
SELECT * FROM
(SELECT empno,
salary,
- count(1) OVER (ORDER BY salary DESC) c
+ count(empno) OVER (ORDER BY salary DESC) c
FROM empsalary) emp
WHERE c <= 3;
SELECT * FROM
(SELECT empno,
salary,
- count(1) OVER (ORDER BY salary DESC) c
+ count(empno) OVER (ORDER BY salary DESC) c
FROM empsalary) emp
WHERE c <= 3;
@@ -1315,13 +1315,13 @@ SELECT empno, depname FROM
FROM empsalary) emp
WHERE rn < 3;
--- likewise with count(1) instead of row_number()
+-- likewise with count(empno) instead of row_number()
EXPLAIN (COSTS OFF)
SELECT * FROM
(SELECT empno,
depname,
salary,
- count(1) OVER (PARTITION BY depname ORDER BY salary DESC) c
+ count(empno) OVER (PARTITION BY depname ORDER BY salary DESC) c
FROM empsalary) emp
WHERE c <= 3;
@@ -1330,7 +1330,7 @@ SELECT * FROM
(SELECT empno,
depname,
salary,
- count(1) OVER (PARTITION BY depname ORDER BY salary DESC) c
+ count(empno) OVER (PARTITION BY depname ORDER BY salary DESC) c
FROM empsalary) emp
WHERE c <= 3;
@@ -1341,7 +1341,7 @@ SELECT * FROM
(SELECT empno,
depname,
salary,
- count(1) OVER () c
+ count(empno) OVER () c
FROM empsalary) emp
WHERE c = 1;
@@ -1349,7 +1349,7 @@ WHERE c = 1;
EXPLAIN (COSTS OFF)
SELECT * FROM
(SELECT *,
- count(1) OVER (PARTITION BY depname || '') c1, -- w1
+ count(salary) OVER (PARTITION BY depname || '') c1, -- w1
row_number() OVER (PARTITION BY depname) rn, -- w2
count(*) OVER (PARTITION BY depname) c2, -- w2
count(*) OVER (PARTITION BY '' || depname) c3, -- w3
@@ -1360,7 +1360,7 @@ SELECT * FROM
-- Ensure we correctly filter out all of the run conditions from each window
SELECT * FROM
(SELECT *,
- count(1) OVER (PARTITION BY depname || '') c1, -- w1
+ count(salary) OVER (PARTITION BY depname || '') c1, -- w1
row_number() OVER (PARTITION BY depname) rn, -- w2
count(*) OVER (PARTITION BY depname) c2, -- w2
count(*) OVER (PARTITION BY '' || depname) c3, -- w3
@@ -1368,6 +1368,23 @@ SELECT * FROM
FROM empsalary
) e WHERE rn <= 1 AND c1 <= 3 AND nt < 2;
+-- Ensure we remove references to reduced outer joins as nulling rels in run
+-- conditions
+EXPLAIN (COSTS OFF)
+SELECT 1 FROM
+ (SELECT ntile(e2.salary) OVER (PARTITION BY e1.depname) AS c
+ FROM empsalary e1 LEFT JOIN empsalary e2 ON TRUE
+ WHERE e1.empno = e2.empno) s
+WHERE s.c = 1;
+
+-- Ensure the run condition optimization is used in cases where the WindowFunc
+-- has a Var from another query level
+EXPLAIN (COSTS OFF)
+SELECT 1 FROM
+ (SELECT ntile(s1.x) OVER () AS c
+ FROM (SELECT (SELECT 1) AS x) AS s1) s
+WHERE s.c = 1;
+
-- Tests to ensure we don't push down the run condition when it's not valid to
-- do so.
@@ -1401,24 +1418,6 @@ SELECT * FROM
FROM empsalary) emp
WHERE c = 1;
--- Ensure we don't use a run condition when the WindowFunc arg contains a Var
-EXPLAIN (COSTS OFF)
-SELECT * FROM
- (SELECT empno,
- salary,
- count(empno) OVER (ORDER BY empno DESC) c
- FROM empsalary) emp
-WHERE c = 1;
-
--- As above but with ntile().
-EXPLAIN (COSTS OFF)
-SELECT * FROM
- (SELECT empno,
- salary,
- ntile(empno::int) OVER (ORDER BY empno DESC) nt
- FROM empsalary) emp
-WHERE nt = 1;
-
-- Ensure we don't use a run condition when the WindowFunc contains subplans
EXPLAIN (COSTS OFF)
SELECT * FROM
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index d551ada325..f32a4cc272 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -3121,6 +3121,7 @@ WindowDef
WindowFunc
WindowFuncExprState
WindowFuncLists
+WindowFuncRunCondition
WindowObject
WindowObjectData
WindowStatePerAgg
--
2.40.1
* Re: BUG #18305: Unexpected error: "WindowFunc not found in subplan target lists" triggered by subqueries
@ 2024-04-25 15:39 Tom Lane <tgl@sss.pgh.pa.us>
parent: David Rowley <dgrowleyml@gmail.com>
0 siblings, 1 reply; 15+ messages in thread
From: Tom Lane @ 2024-04-25 15:39 UTC (permalink / raw)
To: David Rowley <dgrowleyml@gmail.com>; +Cc: Richard Guo <guofenglinux@gmail.com>; zuming.jiang@inf.ethz.ch; pgsql-bugs@lists.postgresql.org
David Rowley <dgrowleyml@gmail.com> writes:
> I've attached a more complete version of the patch (0002) and another
> patch which is what I'd proposed as a fix for the backbranches (0001).
> Note quite a few tests needed to be adjusted because of disabling this
> optimisation.
IIUC, 0002 is meant to be applied on top of 0001, but it reverses
quite a lot of 0001? Please don't commit it like that, it'll just
add a lot of useless thrashing to "git blame" results.
It'd be easier to review this if you presented it as two independent
patches, one for HEAD and one for the back branches.
The fact that you had to use a cheesy "eval_const_expressions(NULL,
..." call in 0001 demonstrates that it was a mistake to not include
PlannerInfo in SupportRequestWFuncMonotonic, as every other planner-
invoked support request has. I realize that we can't change that
in back branches, and that it's no longer immediately necessary
in HEAD either after 0002. But let's learn from experience and
add it to the struct while we're here.
> The 0002 patch will require a cat version bump as it adds a field to
> WindowFunc. Ideally, I'd be applying this fix to master, but I
> imagine some people might feel we should delay applying a fix like
> this until after we branch for v18. Happy to hear people's views on
> that.
catversion bumps are not a problem at this stage --- we will surely
do at least one more for assigned-OID renumbering. I'd rather avoid
the git history thrashing implicit in treating 0002 as a follow-on
patch.
regards, tom lane
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #18305: Unexpected error: "WindowFunc not found in subplan target lists" triggered by subqueries
@ 2024-04-29 21:39 David Rowley <dgrowleyml@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 15+ messages in thread
From: David Rowley @ 2024-04-29 21:39 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Richard Guo <guofenglinux@gmail.com>; zuming.jiang@inf.ethz.ch; pgsql-bugs@lists.postgresql.org
On Fri, 26 Apr 2024 at 03:39, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> It'd be easier to review this if you presented it as two independent
> patches, one for HEAD and one for the back branches.
The attached v2 is the same patch as earlier and is intended for <= v16.
> The fact that you had to use a cheesy "eval_const_expressions(NULL,
> ..." call in 0001 demonstrates that it was a mistake to not include
> PlannerInfo in SupportRequestWFuncMonotonic, as every other planner-
> invoked support request has. I realize that we can't change that
> in back branches, and that it's no longer immediately necessary
> in HEAD either after 0002. But let's learn from experience and
> add it to the struct while we're here.
The correct PlannerInfo to set here would be the one that the
WindowFunc belongs to. The problem is that this code is called from
set_subquery_pathlist before the rel->subroot = subquery_planner. i.e
we've no PlannerInfo to set. Maybe it's worth doing this for
SupportRequestOptimizeWindowClause as a separate patch.
> catversion bumps are not a problem at this stage
The attached v3 is a separate patch for v17 only.
David
Attachments: download all (2 files, .tar.gz)
v2-0001-Disable-run-conditions-for-ntile-var-and-count-va.patch (text/plain, 12.3K) download
From 97812b07cd07bbd65b206f792093d8451ea4fe72 Mon Sep 17 00:00:00 2001
From: David Rowley <dgrowley@gmail.com>
Date: Tue, 23 Apr 2024 14:31:11 +1200
Subject: [PATCH v2 1/2] Disable run conditions for ntile(var) and count(var)
---
src/backend/utils/adt/int8.c | 9 +++
src/backend/utils/adt/windowfuncs.c | 24 +++++--
src/test/regress/expected/window.out | 94 +++++++++++++++-------------
src/test/regress/sql/window.sql | 43 ++++++++-----
4 files changed, 105 insertions(+), 65 deletions(-)
diff --git a/src/backend/utils/adt/int8.c b/src/backend/utils/adt/int8.c
index 54fa3bc379..e7b1d389b3 100644
--- a/src/backend/utils/adt/int8.c
+++ b/src/backend/utils/adt/int8.c
@@ -832,6 +832,15 @@ int8inc_support(PG_FUNCTION_ARGS)
SupportRequestWFuncMonotonic *req = (SupportRequestWFuncMonotonic *) rawreq;
MonotonicFunction monotonic = MONOTONICFUNC_NONE;
int frameOptions = req->window_clause->frameOptions;
+ WindowFunc *wfunc = req->window_func;
+
+ if (list_length(wfunc->args) == 1)
+ {
+ Node *expr = eval_const_expressions(NULL, linitial(wfunc->args));
+
+ if (!IsA(expr, Const))
+ PG_RETURN_POINTER(NULL);
+ }
/* No ORDER BY clause then all rows are peers */
if (req->window_clause->orderClause == NIL)
diff --git a/src/backend/utils/adt/windowfuncs.c b/src/backend/utils/adt/windowfuncs.c
index 473c61569f..b49e48109d 100644
--- a/src/backend/utils/adt/windowfuncs.c
+++ b/src/backend/utils/adt/windowfuncs.c
@@ -16,6 +16,7 @@
#include "nodes/parsenodes.h"
#include "nodes/supportnodes.h"
#include "utils/fmgrprotos.h"
+#include "optimizer/optimizer.h"
#include "windowapi.h"
/*
@@ -487,13 +488,24 @@ window_ntile_support(PG_FUNCTION_ARGS)
if (IsA(rawreq, SupportRequestWFuncMonotonic))
{
SupportRequestWFuncMonotonic *req = (SupportRequestWFuncMonotonic *) rawreq;
+ WindowFunc *wfunc = req->window_func;
- /*
- * ntile() is monotonically increasing as the number of buckets cannot
- * change after the first call
- */
- req->monotonic = MONOTONICFUNC_INCREASING;
- PG_RETURN_POINTER(req);
+ if (list_length(wfunc->args) == 1)
+ {
+ Node *expr = eval_const_expressions(NULL, linitial(wfunc->args));
+
+ if (IsA(expr, Const))
+ {
+ /*
+ * ntile() is monotonically increasing as the number of
+ * buckets cannot change after the first call
+ */
+ req->monotonic = MONOTONICFUNC_INCREASING;
+ PG_RETURN_POINTER(req);
+ }
+ }
+
+ PG_RETURN_POINTER(NULL);
}
if (IsA(rawreq, SupportRequestOptimizeWindowClause))
diff --git a/src/test/regress/expected/window.out b/src/test/regress/expected/window.out
index e46710cf31..2f2cb112b1 100644
--- a/src/test/regress/expected/window.out
+++ b/src/test/regress/expected/window.out
@@ -3954,13 +3954,13 @@ EXPLAIN (COSTS OFF)
SELECT * FROM
(SELECT empno,
salary,
- count(empno) OVER (ORDER BY salary DESC) c
+ count(1) OVER (ORDER BY salary DESC) c
FROM empsalary) emp
WHERE c <= 3;
- QUERY PLAN
----------------------------------------------------------
+ QUERY PLAN
+-------------------------------------------
WindowAgg
- Run Condition: (count(empsalary.empno) OVER (?) <= 3)
+ Run Condition: (count(1) OVER (?) <= 3)
-> Sort
Sort Key: empsalary.salary DESC
-> Seq Scan on empsalary
@@ -3969,7 +3969,7 @@ WHERE c <= 3;
SELECT * FROM
(SELECT empno,
salary,
- count(empno) OVER (ORDER BY salary DESC) c
+ count(1) OVER (ORDER BY salary DESC) c
FROM empsalary) emp
WHERE c <= 3;
empno | salary | c
@@ -4081,19 +4081,19 @@ WHERE rn < 3;
-> Seq Scan on empsalary
(6 rows)
--- likewise with count(empno) instead of row_number()
+-- likewise with count(1) instead of row_number()
EXPLAIN (COSTS OFF)
SELECT * FROM
(SELECT empno,
depname,
salary,
- count(empno) OVER (PARTITION BY depname ORDER BY salary DESC) c
+ count(1) OVER (PARTITION BY depname ORDER BY salary DESC) c
FROM empsalary) emp
WHERE c <= 3;
QUERY PLAN
------------------------------------------------------------
WindowAgg
- Run Condition: (count(empsalary.empno) OVER (?) <= 3)
+ Run Condition: (count(1) OVER (?) <= 3)
-> Sort
Sort Key: empsalary.depname, empsalary.salary DESC
-> Seq Scan on empsalary
@@ -4104,7 +4104,7 @@ SELECT * FROM
(SELECT empno,
depname,
salary,
- count(empno) OVER (PARTITION BY depname ORDER BY salary DESC) c
+ count(1) OVER (PARTITION BY depname ORDER BY salary DESC) c
FROM empsalary) emp
WHERE c <= 3;
empno | depname | salary | c
@@ -4126,13 +4126,13 @@ SELECT * FROM
(SELECT empno,
depname,
salary,
- count(empno) OVER () c
+ count(1) OVER () c
FROM empsalary) emp
WHERE c = 1;
- QUERY PLAN
---------------------------------------------------------
+ QUERY PLAN
+------------------------------------------
WindowAgg
- Run Condition: (count(empsalary.empno) OVER (?) = 1)
+ Run Condition: (count(1) OVER (?) = 1)
-> Seq Scan on empsalary
(3 rows)
@@ -4140,7 +4140,7 @@ WHERE c = 1;
EXPLAIN (COSTS OFF)
SELECT * FROM
(SELECT *,
- count(salary) OVER (PARTITION BY depname || '') c1, -- w1
+ count(1) OVER (PARTITION BY depname || '') c1, -- w1
row_number() OVER (PARTITION BY depname) rn, -- w2
count(*) OVER (PARTITION BY depname) c2, -- w2
count(*) OVER (PARTITION BY '' || depname) c3, -- w3
@@ -4152,7 +4152,7 @@ SELECT * FROM
Subquery Scan on e
-> WindowAgg
Filter: (((row_number() OVER (?)) <= 1) AND ((ntile(2) OVER (?)) < 2))
- Run Condition: (count(empsalary.salary) OVER (?) <= 3)
+ Run Condition: (count(1) OVER (?) <= 3)
-> Sort
Sort Key: (((empsalary.depname)::text || ''::text))
-> WindowAgg
@@ -4168,7 +4168,7 @@ SELECT * FROM
-- Ensure we correctly filter out all of the run conditions from each window
SELECT * FROM
(SELECT *,
- count(salary) OVER (PARTITION BY depname || '') c1, -- w1
+ count(1) OVER (PARTITION BY depname || '') c1, -- w1
row_number() OVER (PARTITION BY depname) rn, -- w2
count(*) OVER (PARTITION BY depname) c2, -- w2
count(*) OVER (PARTITION BY '' || depname) c3, -- w3
@@ -4181,32 +4181,6 @@ SELECT * FROM
sales | 3 | 4800 | 08-01-2007 | 3 | 1 | 3 | 3 | 1
(2 rows)
--- Ensure we remove references to reduced outer joins as nulling rels in run
--- conditions
-EXPLAIN (COSTS OFF)
-SELECT 1 FROM
- (SELECT ntile(e2.salary) OVER (PARTITION BY e1.depname) AS c
- FROM empsalary e1 LEFT JOIN empsalary e2 ON TRUE
- WHERE e1.empno = e2.empno) s
-WHERE s.c = 1;
- QUERY PLAN
----------------------------------------------------------
- Subquery Scan on s
- Filter: (s.c = 1)
- -> WindowAgg
- Run Condition: (ntile(e2.salary) OVER (?) <= 1)
- -> Sort
- Sort Key: e1.depname
- -> Merge Join
- Merge Cond: (e1.empno = e2.empno)
- -> Sort
- Sort Key: e1.empno
- -> Seq Scan on empsalary e1
- -> Sort
- Sort Key: e2.empno
- -> Seq Scan on empsalary e2
-(14 rows)
-
-- Tests to ensure we don't push down the run condition when it's not valid to
-- do so.
-- Ensure we don't push down when the frame options show that the window
@@ -4266,6 +4240,42 @@ WHERE c = 1;
-> Seq Scan on empsalary
(6 rows)
+-- Ensure we don't use a run condition when the WindowFunc arg contains a Var
+EXPLAIN (COSTS OFF)
+SELECT * FROM
+ (SELECT empno,
+ salary,
+ count(empno) OVER (ORDER BY empno DESC) c
+ FROM empsalary) emp
+WHERE c = 1;
+ QUERY PLAN
+----------------------------------------------
+ Subquery Scan on emp
+ Filter: (emp.c = 1)
+ -> WindowAgg
+ -> Sort
+ Sort Key: empsalary.empno DESC
+ -> Seq Scan on empsalary
+(6 rows)
+
+-- As above but with ntile().
+EXPLAIN (COSTS OFF)
+SELECT * FROM
+ (SELECT empno,
+ salary,
+ ntile(empno::int) OVER (ORDER BY empno DESC) nt
+ FROM empsalary) emp
+WHERE nt = 1;
+ QUERY PLAN
+----------------------------------------------
+ Subquery Scan on emp
+ Filter: (emp.nt = 1)
+ -> WindowAgg
+ -> Sort
+ Sort Key: empsalary.empno DESC
+ -> Seq Scan on empsalary
+(6 rows)
+
-- Ensure we don't use a run condition when the WindowFunc contains subplans
EXPLAIN (COSTS OFF)
SELECT * FROM
diff --git a/src/test/regress/sql/window.sql b/src/test/regress/sql/window.sql
index 678aa1a21c..284f47b514 100644
--- a/src/test/regress/sql/window.sql
+++ b/src/test/regress/sql/window.sql
@@ -1252,14 +1252,14 @@ EXPLAIN (COSTS OFF)
SELECT * FROM
(SELECT empno,
salary,
- count(empno) OVER (ORDER BY salary DESC) c
+ count(1) OVER (ORDER BY salary DESC) c
FROM empsalary) emp
WHERE c <= 3;
SELECT * FROM
(SELECT empno,
salary,
- count(empno) OVER (ORDER BY salary DESC) c
+ count(1) OVER (ORDER BY salary DESC) c
FROM empsalary) emp
WHERE c <= 3;
@@ -1315,13 +1315,13 @@ SELECT empno, depname FROM
FROM empsalary) emp
WHERE rn < 3;
--- likewise with count(empno) instead of row_number()
+-- likewise with count(1) instead of row_number()
EXPLAIN (COSTS OFF)
SELECT * FROM
(SELECT empno,
depname,
salary,
- count(empno) OVER (PARTITION BY depname ORDER BY salary DESC) c
+ count(1) OVER (PARTITION BY depname ORDER BY salary DESC) c
FROM empsalary) emp
WHERE c <= 3;
@@ -1330,7 +1330,7 @@ SELECT * FROM
(SELECT empno,
depname,
salary,
- count(empno) OVER (PARTITION BY depname ORDER BY salary DESC) c
+ count(1) OVER (PARTITION BY depname ORDER BY salary DESC) c
FROM empsalary) emp
WHERE c <= 3;
@@ -1341,7 +1341,7 @@ SELECT * FROM
(SELECT empno,
depname,
salary,
- count(empno) OVER () c
+ count(1) OVER () c
FROM empsalary) emp
WHERE c = 1;
@@ -1349,7 +1349,7 @@ WHERE c = 1;
EXPLAIN (COSTS OFF)
SELECT * FROM
(SELECT *,
- count(salary) OVER (PARTITION BY depname || '') c1, -- w1
+ count(1) OVER (PARTITION BY depname || '') c1, -- w1
row_number() OVER (PARTITION BY depname) rn, -- w2
count(*) OVER (PARTITION BY depname) c2, -- w2
count(*) OVER (PARTITION BY '' || depname) c3, -- w3
@@ -1360,7 +1360,7 @@ SELECT * FROM
-- Ensure we correctly filter out all of the run conditions from each window
SELECT * FROM
(SELECT *,
- count(salary) OVER (PARTITION BY depname || '') c1, -- w1
+ count(1) OVER (PARTITION BY depname || '') c1, -- w1
row_number() OVER (PARTITION BY depname) rn, -- w2
count(*) OVER (PARTITION BY depname) c2, -- w2
count(*) OVER (PARTITION BY '' || depname) c3, -- w3
@@ -1368,15 +1368,6 @@ SELECT * FROM
FROM empsalary
) e WHERE rn <= 1 AND c1 <= 3 AND nt < 2;
--- Ensure we remove references to reduced outer joins as nulling rels in run
--- conditions
-EXPLAIN (COSTS OFF)
-SELECT 1 FROM
- (SELECT ntile(e2.salary) OVER (PARTITION BY e1.depname) AS c
- FROM empsalary e1 LEFT JOIN empsalary e2 ON TRUE
- WHERE e1.empno = e2.empno) s
-WHERE s.c = 1;
-
-- Tests to ensure we don't push down the run condition when it's not valid to
-- do so.
@@ -1410,6 +1401,24 @@ SELECT * FROM
FROM empsalary) emp
WHERE c = 1;
+-- Ensure we don't use a run condition when the WindowFunc arg contains a Var
+EXPLAIN (COSTS OFF)
+SELECT * FROM
+ (SELECT empno,
+ salary,
+ count(empno) OVER (ORDER BY empno DESC) c
+ FROM empsalary) emp
+WHERE c = 1;
+
+-- As above but with ntile().
+EXPLAIN (COSTS OFF)
+SELECT * FROM
+ (SELECT empno,
+ salary,
+ ntile(empno::int) OVER (ORDER BY empno DESC) nt
+ FROM empsalary) emp
+WHERE nt = 1;
+
-- Ensure we don't use a run condition when the WindowFunc contains subplans
EXPLAIN (COSTS OFF)
SELECT * FROM
--
2.40.1
v3-0001-Fix-query-pullup-issue-with-WindowClause-runCondi.patch (text/plain, 19.6K) download
From 389e194f060ba68c89d334cf4b5f68475eb83e15 Mon Sep 17 00:00:00 2001
From: David Rowley <dgrowley@gmail.com>
Date: Mon, 29 Apr 2024 14:33:02 +1200
Subject: [PATCH v3] Fix query pullup issue with WindowClause runCondition
When attempting to push quals into subqueries, the code added in
9d9c02ccd attempted to transform OpExprs which reference momotonic
WindowFuncs into quals that can be used to short-circuit execution.
Unfortunately that optimization is broken as it's performed before we
perform subquery pullup and duplicating the WindowFunc in the
WindowClauses runCondition causes multiple issues when the WindowFunc
contains arguments containing Vars resulting in errors such as:
ERROR: WindowFunc not found in subplan target lists
To fix this, we change the node representation of these run conditions
and instead of storing an OpExpr containing the WindowFunc in a list
inside WindowClause, we now store a new node type named
WindowFuncRunCondition within a new field in the WindowFunc. These get
transformed into OpExprs later in planning once subquery pullup has been
performed.
Cat version bump due to new node type and modifying WindowFunc struct.
Reported-by: Zuming Jiang
Discussion: https://postgr.es/m/18305-33c49b4c830b37b3%40postgresql.org
---
src/backend/nodes/nodeFuncs.c | 26 ++++++++---
src/backend/optimizer/path/allpaths.c | 36 +++++----------
src/backend/optimizer/plan/createplan.c | 2 +-
src/backend/optimizer/plan/planner.c | 54 ++++++++++++++++++-----
src/backend/optimizer/prep/prepjointree.c | 8 ----
src/backend/optimizer/util/clauses.c | 1 +
src/backend/optimizer/util/pathnode.c | 3 ++
src/backend/parser/parse_clause.c | 1 -
src/backend/parser/parse_expr.c | 1 +
src/backend/parser/parse_func.c | 1 +
src/include/nodes/parsenodes.h | 2 -
src/include/nodes/pathnodes.h | 1 +
src/include/nodes/primnodes.h | 30 +++++++++++++
src/include/optimizer/pathnode.h | 1 +
src/test/regress/expected/window.out | 18 ++++++++
src/test/regress/sql/window.sql | 8 ++++
src/tools/pgindent/typedefs.list | 1 +
17 files changed, 142 insertions(+), 52 deletions(-)
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index e1df1894b6..3db4a42dd7 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -2163,6 +2163,16 @@ expression_tree_walker_impl(Node *node,
return true;
if (WALK(expr->aggfilter))
return true;
+ if (WALK(expr->runCondition))
+ return true;
+ }
+ break;
+ case T_WindowFuncRunCondition:
+ {
+ WindowFuncRunCondition *expr = (WindowFuncRunCondition *) node;
+
+ if (WALK(expr->arg))
+ return true;
}
break;
case T_SubscriptingRef:
@@ -2400,8 +2410,6 @@ expression_tree_walker_impl(Node *node,
return true;
if (WALK(wc->endOffset))
return true;
- if (WALK(wc->runCondition))
- return true;
}
break;
case T_CTECycleClause:
@@ -2752,8 +2760,6 @@ query_tree_walker_impl(Query *query,
return true;
if (WALK(wc->endOffset))
return true;
- if (WALK(wc->runCondition))
- return true;
}
}
@@ -3053,6 +3059,16 @@ expression_tree_mutator_impl(Node *node,
return (Node *) newnode;
}
break;
+ case T_WindowFuncRunCondition:
+ {
+ WindowFuncRunCondition *expr = (WindowFuncRunCondition *) node;
+ WindowFuncRunCondition *newnode;
+
+ FLATCOPY(newnode, expr, WindowFuncRunCondition);
+ MUTATE(newnode->arg, expr->arg, Expr *);
+ return (Node *) newnode;
+ }
+ break;
case T_SubscriptingRef:
{
SubscriptingRef *sbsref = (SubscriptingRef *) node;
@@ -3466,7 +3482,6 @@ expression_tree_mutator_impl(Node *node,
MUTATE(newnode->orderClause, wc->orderClause, List *);
MUTATE(newnode->startOffset, wc->startOffset, Node *);
MUTATE(newnode->endOffset, wc->endOffset, Node *);
- MUTATE(newnode->runCondition, wc->runCondition, List *);
return (Node *) newnode;
}
break;
@@ -3799,7 +3814,6 @@ query_tree_mutator_impl(Query *query,
FLATCOPY(newnode, wc, WindowClause);
MUTATE(newnode->startOffset, wc->startOffset, Node *);
MUTATE(newnode->endOffset, wc->endOffset, Node *);
- MUTATE(newnode->runCondition, wc->runCondition, List *);
resultlist = lappend(resultlist, (Node *) newnode);
}
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index cc51ae1757..f21ae3ecda 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -2205,7 +2205,7 @@ set_dummy_rel_pathlist(RelOptInfo *rel)
* the run condition will handle all of the required filtering.
*
* Returns true if 'opexpr' was found to be useful and was added to the
- * WindowClauses runCondition. We also set *keep_original accordingly and add
+ * WindowFunc's runCondition. We also set *keep_original accordingly and add
* 'attno' to *run_cond_attrs offset by FirstLowInvalidHeapAttributeNumber.
* If the 'opexpr' cannot be used then we set *keep_original to true and
* return false.
@@ -2358,7 +2358,7 @@ find_window_run_conditions(Query *subquery, RangeTblEntry *rte, Index rti,
*keep_original = true;
runopexpr = opexpr;
- /* determine the operator to use for the runCondition qual */
+ /* determine the operator to use for the WindowFuncRunCondition */
runoperator = get_opfamily_member(opinfo->opfamily_id,
opinfo->oplefttype,
opinfo->oprighttype,
@@ -2369,27 +2369,15 @@ find_window_run_conditions(Query *subquery, RangeTblEntry *rte, Index rti,
if (runopexpr != NULL)
{
- Expr *newexpr;
+ WindowFuncRunCondition *runcond;
- /*
- * Build the qual required for the run condition keeping the
- * WindowFunc on the same side as it was originally.
- */
- if (wfunc_left)
- newexpr = make_opclause(runoperator,
- runopexpr->opresulttype,
- runopexpr->opretset, (Expr *) wfunc,
- otherexpr, runopexpr->opcollid,
- runopexpr->inputcollid);
- else
- newexpr = make_opclause(runoperator,
- runopexpr->opresulttype,
- runopexpr->opretset,
- otherexpr, (Expr *) wfunc,
- runopexpr->opcollid,
- runopexpr->inputcollid);
+ runcond = makeNode(WindowFuncRunCondition);
+ runcond->opno = runoperator;
+ runcond->inputcollid = runopexpr->inputcollid;
+ runcond->wfunc_left = wfunc_left;
+ runcond->arg = copyObject(otherexpr);
- wclause->runCondition = lappend(wclause->runCondition, newexpr);
+ wfunc->runCondition = lappend(wfunc->runCondition, runcond);
/* record that this attno was used in a run condition */
*run_cond_attrs = bms_add_member(*run_cond_attrs,
@@ -2403,9 +2391,9 @@ find_window_run_conditions(Query *subquery, RangeTblEntry *rte, Index rti,
/*
* check_and_push_window_quals
- * Check if 'clause' is a qual that can be pushed into a WindowFunc's
- * WindowClause as a 'runCondition' qual. These, when present, allow
- * some unnecessary work to be skipped during execution.
+ * Check if 'clause' is a qual that can be pushed into a WindowFunc
+ * as a 'runCondition' qual. These, when present, allow some unnecessary
+ * work to be skipped during execution.
*
* 'run_cond_attrs' will be populated with all targetlist resnos of subquery
* targets (offset by FirstLowInvalidHeapAttributeNumber) that we pushed
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 3b77886567..6b64c4a362 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -2699,7 +2699,7 @@ create_windowagg_plan(PlannerInfo *root, WindowAggPath *best_path)
wc->inRangeColl,
wc->inRangeAsc,
wc->inRangeNullsFirst,
- wc->runCondition,
+ best_path->runCondition,
best_path->qual,
best_path->topwindow,
subplan);
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 5320da51a0..b149b5690d 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -870,9 +870,6 @@ subquery_planner(PlannerGlobal *glob, Query *parse, PlannerInfo *parent_root,
EXPRKIND_LIMIT);
wc->endOffset = preprocess_expression(root, wc->endOffset,
EXPRKIND_LIMIT);
- wc->runCondition = (List *) preprocess_expression(root,
- (Node *) wc->runCondition,
- EXPRKIND_TARGET);
}
parse->limitOffset = preprocess_expression(root, parse->limitOffset,
@@ -4527,9 +4524,11 @@ create_one_window_path(PlannerInfo *root,
{
WindowClause *wc = lfirst_node(WindowClause, l);
List *window_pathkeys;
+ List *runcondition = NIL;
int presorted_keys;
bool is_sorted;
bool topwindow;
+ ListCell *lc2;
window_pathkeys = make_pathkeys_for_window(root,
wc,
@@ -4577,7 +4576,6 @@ create_one_window_path(PlannerInfo *root,
* we do need to account for the increase in tlist width.
*/
int64 tuple_width = window_target->width;
- ListCell *lc2;
window_target = copy_pathtarget(window_target);
foreach(lc2, wflists->windowFuncs[wc->winref])
@@ -4599,17 +4597,53 @@ create_one_window_path(PlannerInfo *root,
topwindow = foreach_current_index(l) == list_length(activeWindows) - 1;
/*
- * Accumulate all of the runConditions from each intermediate
- * WindowClause. The top-level WindowAgg must pass these as a qual so
- * that it filters out unwanted tuples correctly.
+ * Collect the WindowFuncRunConditions from each WindowFunc and
+ * convert them into OpExprs
*/
- if (!topwindow)
- topqual = list_concat(topqual, wc->runCondition);
+ foreach(lc2, wflists->windowFuncs[wc->winref])
+ {
+ ListCell *lc3;
+ WindowFunc *wfunc = lfirst_node(WindowFunc, lc2);
+
+ foreach(lc3, wfunc->runCondition)
+ {
+ WindowFuncRunCondition *rc =
+ lfirst_node(WindowFuncRunCondition, lc3);
+ Expr *opexpr;
+ Expr *leftop;
+ Expr *rightop;
+
+ if (rc->wfunc_left)
+ {
+ leftop = (Expr *) copyObject(wfunc);
+ rightop = copyObject(rc->arg);
+ }
+ else
+ {
+ leftop = copyObject(rc->arg);
+ rightop = (Expr *) copyObject(wfunc);
+ }
+
+ opexpr = make_opclause(rc->opno,
+ BOOLOID,
+ false,
+ leftop,
+ rightop,
+ InvalidOid,
+ rc->inputcollid);
+
+ runcondition = lappend(runcondition, opexpr);
+
+ if (!topwindow)
+ topqual = lappend(topqual, opexpr);
+ }
+ }
path = (Path *)
create_windowagg_path(root, window_rel, path, window_target,
wflists->windowFuncs[wc->winref],
- wc, topwindow ? topqual : NIL, topwindow);
+ runcondition, wc,
+ topwindow ? topqual : NIL, topwindow);
}
add_path(window_rel, path);
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index 41da670f15..5482ab85a7 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -2175,14 +2175,6 @@ perform_pullup_replace_vars(PlannerInfo *root,
parse->returningList = (List *)
pullup_replace_vars((Node *) parse->returningList, rvcontext);
- foreach(lc, parse->windowClause)
- {
- WindowClause *wc = lfirst_node(WindowClause, lc);
-
- if (wc->runCondition != NIL)
- wc->runCondition = (List *)
- pullup_replace_vars((Node *) wc->runCondition, rvcontext);
- }
if (parse->onConflict)
{
parse->onConflict->onConflictSet = (List *)
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 59487cbd79..b4e085e9d4 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -2566,6 +2566,7 @@ eval_const_expressions_mutator(Node *node,
newexpr->inputcollid = expr->inputcollid;
newexpr->args = args;
newexpr->aggfilter = aggfilter;
+ newexpr->runCondition = expr->runCondition;
newexpr->winref = expr->winref;
newexpr->winstar = expr->winstar;
newexpr->winagg = expr->winagg;
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 3cf1dac087..3491c3af1c 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -3471,6 +3471,7 @@ create_minmaxagg_path(PlannerInfo *root,
* 'subpath' is the path representing the source of data
* 'target' is the PathTarget to be computed
* 'windowFuncs' is a list of WindowFunc structs
+ * 'runCondition' is a list of OpExprs to short-circuit WindowAgg execution
* 'winclause' is a WindowClause that is common to all the WindowFuncs
* 'qual' WindowClause.runconditions from lower-level WindowAggPaths.
* Must always be NIL when topwindow == false
@@ -3486,6 +3487,7 @@ create_windowagg_path(PlannerInfo *root,
Path *subpath,
PathTarget *target,
List *windowFuncs,
+ List *runCondition,
WindowClause *winclause,
List *qual,
bool topwindow)
@@ -3510,6 +3512,7 @@ create_windowagg_path(PlannerInfo *root,
pathnode->subpath = subpath;
pathnode->winclause = winclause;
pathnode->qual = qual;
+ pathnode->runCondition = runCondition;
pathnode->topwindow = topwindow;
/*
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index 4fc5fc87e0..8118036495 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -2956,7 +2956,6 @@ transformWindowDefinitions(ParseState *pstate,
rangeopfamily, rangeopcintype,
&wc->endInRangeFunc,
windef->endOffset);
- wc->runCondition = NIL;
wc->winref = winref;
result = lappend(result, wc);
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 1c1c86aa3e..aba3546ed1 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -3826,6 +3826,7 @@ transformJsonAggConstructor(ParseState *pstate, JsonAggConstructor *agg_ctor,
/* wincollid and inputcollid will be set by parse_collate.c */
wfunc->args = args;
wfunc->aggfilter = aggfilter;
+ wfunc->runCondition = NIL;
/* winref will be set by transformWindowFuncCall */
wfunc->winstar = false;
wfunc->winagg = true;
diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index 0cbc950c95..9b23344a3b 100644
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -834,6 +834,7 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
wfunc->winstar = agg_star;
wfunc->winagg = (fdresult == FUNCDETAIL_AGGREGATE);
wfunc->aggfilter = agg_filter;
+ wfunc->runCondition = NIL;
wfunc->location = location;
/*
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index af80a5d38e..3ca06fc3af 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1549,8 +1549,6 @@ typedef struct WindowClause
int frameOptions; /* frame_clause options, see WindowDef */
Node *startOffset; /* expression for starting bound, if any */
Node *endOffset; /* expression for ending bound, if any */
- /* qual to help short-circuit execution */
- List *runCondition pg_node_attr(query_jumble_ignore);
/* in_range function for startOffset */
Oid startInRangeFunc pg_node_attr(query_jumble_ignore);
/* in_range function for endOffset */
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index b8141f141a..99bd371e0a 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -2308,6 +2308,7 @@ typedef struct WindowAggPath
Path *subpath; /* path representing input source */
WindowClause *winclause; /* WindowClause we'll be using */
List *qual; /* lower-level WindowAgg runconditions */
+ List *runCondition; /* OpExpr List to short-circuit execution */
bool topwindow; /* false for all apart from the WindowAgg
* that's closest to the root of the plan */
} WindowAggPath;
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index 247cecb4b4..1ea5b65f53 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -575,6 +575,8 @@ typedef struct WindowFunc
List *args;
/* FILTER expression, if any */
Expr *aggfilter;
+ /* List of WindowFuncRunConditions to help short-circuit execution */
+ List *runCondition pg_node_attr(query_jumble_ignore);
/* index of associated WindowClause */
Index winref;
/* true if argument list was really '*' */
@@ -585,6 +587,34 @@ typedef struct WindowFunc
ParseLoc location;
} WindowFunc;
+/*
+ * WindowFuncRunCondition
+ *
+ * Represents intermediate OpExprs which will be used by WindowAgg to
+ * short-circuit execution.
+ */
+typedef struct WindowFuncRunCondition
+{
+ Expr xpr;
+
+ /* PG_OPERATOR OID of the operator */
+ Oid opno;
+ /* OID of collation that operator should use */
+ Oid inputcollid pg_node_attr(query_jumble_ignore);
+
+ /*
+ * true of WindowFunc belongs on the left of the resulting OpExpr or false
+ * if the WindowFunc is on the right.
+ */
+ bool wfunc_left;
+
+ /*
+ * The Expr being compared to the WindowFunc to use in the OpExpr in the
+ * WindowAgg's runCondition
+ */
+ Expr *arg;
+} WindowFuncRunCondition;
+
/*
* MergeSupportFunc
*
diff --git a/src/include/optimizer/pathnode.h b/src/include/optimizer/pathnode.h
index c5c4756b0f..112e7c23d4 100644
--- a/src/include/optimizer/pathnode.h
+++ b/src/include/optimizer/pathnode.h
@@ -250,6 +250,7 @@ extern WindowAggPath *create_windowagg_path(PlannerInfo *root,
Path *subpath,
PathTarget *target,
List *windowFuncs,
+ List *runCondition,
WindowClause *winclause,
List *qual,
bool topwindow);
diff --git a/src/test/regress/expected/window.out b/src/test/regress/expected/window.out
index e46710cf31..ae4e8851f8 100644
--- a/src/test/regress/expected/window.out
+++ b/src/test/regress/expected/window.out
@@ -4207,6 +4207,24 @@ WHERE s.c = 1;
-> Seq Scan on empsalary e2
(14 rows)
+-- Ensure the run condition optimization is used in cases where the WindowFunc
+-- has a Var from another query level
+EXPLAIN (COSTS OFF)
+SELECT 1 FROM
+ (SELECT ntile(s1.x) OVER () AS c
+ FROM (SELECT (SELECT 1) AS x) AS s1) s
+WHERE s.c = 1;
+ QUERY PLAN
+-----------------------------------------------------------------
+ Subquery Scan on s
+ Filter: (s.c = 1)
+ -> WindowAgg
+ Run Condition: (ntile((InitPlan 1).col1) OVER (?) <= 1)
+ InitPlan 1
+ -> Result
+ -> Result
+(7 rows)
+
-- Tests to ensure we don't push down the run condition when it's not valid to
-- do so.
-- Ensure we don't push down when the frame options show that the window
diff --git a/src/test/regress/sql/window.sql b/src/test/regress/sql/window.sql
index 678aa1a21c..6de5493b05 100644
--- a/src/test/regress/sql/window.sql
+++ b/src/test/regress/sql/window.sql
@@ -1377,6 +1377,14 @@ SELECT 1 FROM
WHERE e1.empno = e2.empno) s
WHERE s.c = 1;
+-- Ensure the run condition optimization is used in cases where the WindowFunc
+-- has a Var from another query level
+EXPLAIN (COSTS OFF)
+SELECT 1 FROM
+ (SELECT ntile(s1.x) OVER () AS c
+ FROM (SELECT (SELECT 1) AS x) AS s1) s
+WHERE s.c = 1;
+
-- Tests to ensure we don't push down the run condition when it's not valid to
-- do so.
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index e10ff28ee5..eee989bba1 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -3120,6 +3120,7 @@ WindowDef
WindowFunc
WindowFuncExprState
WindowFuncLists
+WindowFuncRunCondition
WindowObject
WindowObjectData
WindowStatePerAgg
--
2.40.1
* Re: BUG #18305: Unexpected error: "WindowFunc not found in subplan target lists" triggered by subqueries
@ 2024-04-30 23:30 David Rowley <dgrowleyml@gmail.com>
parent: David Rowley <dgrowleyml@gmail.com>
0 siblings, 1 reply; 15+ messages in thread
From: David Rowley @ 2024-04-30 23:30 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Richard Guo <guofenglinux@gmail.com>; zuming.jiang@inf.ethz.ch; pgsql-bugs@lists.postgresql.org
On Tue, 30 Apr 2024 at 09:39, David Rowley <dgrowleyml@gmail.com> wrote:
> The attached v2 is the same patch as earlier and is intended for <= v16.
I want to commit this one so it makes it into the next minor version
releases. It's not very complex, so plan to push it later today.
I'll hold off with the v17 version for a while.
David
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: BUG #18305: Unexpected error: "WindowFunc not found in subplan target lists" triggered by subqueries
@ 2024-05-05 00:55 David Rowley <dgrowleyml@gmail.com>
parent: David Rowley <dgrowleyml@gmail.com>
0 siblings, 0 replies; 15+ messages in thread
From: David Rowley @ 2024-05-05 00:55 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Richard Guo <guofenglinux@gmail.com>; zuming.jiang@inf.ethz.ch; pgsql-bugs@lists.postgresql.org
On Wed, 1 May 2024 at 11:30, David Rowley <dgrowleyml@gmail.com> wrote:
> I'll hold off with the v17 version for a while.
I've now pushed the fix for v17.
David
^ permalink raw reply [nested|flat] 15+ messages in thread
end of thread, other threads:[~2024-05-05 00:55 UTC | newest]
Thread overview: 15+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-01-22 12:15 BUG #18305: Unexpected error: "WindowFunc not found in subplan target lists" triggered by subqueries PG Bug reporting form <noreply@postgresql.org>
2024-01-22 15:44 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-01-23 10:41 ` Richard Guo <guofenglinux@gmail.com>
2024-01-23 13:52 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-01-23 19:34 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-01-25 05:14 ` David Rowley <dgrowleyml@gmail.com>
2024-01-26 12:02 ` David Rowley <dgrowleyml@gmail.com>
2024-03-22 08:55 ` Richard Guo <guofenglinux@gmail.com>
2024-04-15 03:42 ` David Rowley <dgrowleyml@gmail.com>
2024-03-22 10:47 ` Richard Guo <guofenglinux@gmail.com>
2024-04-24 02:05 ` David Rowley <dgrowleyml@gmail.com>
2024-04-25 15:39 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-04-29 21:39 ` David Rowley <dgrowleyml@gmail.com>
2024-04-30 23:30 ` David Rowley <dgrowleyml@gmail.com>
2024-05-05 00:55 ` David Rowley <dgrowleyml@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox