Re: Window Function "Run Conditions" - Mailing list pgsql-hackers

On Thu, Aug 19, 2021 at 2:35 PM David Rowley <dgrowleyml@gmail.com> wrote:
>
> On Thu, 19 Aug 2021 at 00:20, Andy Fan <zhihui.fan1213@gmail.com> wrote:
> > In the current master, the result is:
> >
> >  empno | salary | c | dr
> > -------+--------+---+----
> >      8 |   6000 | 4 |  1
>
> > In the patched version， the result is:
> >
> >  empno | salary | c | dr
> > -------+--------+---+----
> >      8 |   6000 | 1 |  1
>
> Thanks for taking it for a spin.
>
> That's a bit unfortunate.  I don't immediately see how to fix it other
> than to restrict the optimisation to only apply when there's a single
> WindowClause. It might be possible to relax it further and only apply
> if it's the final window clause to be evaluated, but in those cases,
> the savings are likely to be much less anyway as some previous
> WindowAgg will have exhausted all rows from its subplan.

I am trying to hack the select_active_windows function to make
sure the WindowClause with Run Condition clause to be the last one
to evaluate (we also need to consider more than 1 window func has
run condition), at that time, the run condition clause is ready already.

However there are two troubles in this direction: a).  This may conflict
with "the windows that need the same sorting are adjacent in the list."
b). "when two or more windows are order-equivalent then all peer rows
must be presented in the same order in all of them. .. (See General Rule 4 of
<window clause> in SQL2008 - SQL2016.)"

In summary, I am not sure if it is correct to change the execution Order
of WindowAgg freely.

>   Likely
> restricting it to only working if there's 1 WindowClause would be fine
> as for the people using row_number() for a top-N type query, there's
> most likely only going to be 1 WindowClause.
>

This sounds practical.  And I suggest the following small changes.
(just check the partitionClause before the prosupport)

@@ -2133,20 +2133,22 @@ find_window_run_conditions(Query *subquery,
RangeTblEntry *rte, Index rti,

        *keep_original = true;

-       prosupport = get_func_support(wfunc->winfnoid);
-
-       /* Check if there's a support function for 'wfunc' */
-       if (!OidIsValid(prosupport))
-               return false;
-
        /*
         * Currently the WindowAgg node just stop when the run condition is no
         * longer true.  If there is a PARTITION BY clause then we cannot just
         * stop as other partitions still need to be processed.
         */
+
+       /* Check this first since window function with a partition
clause is common*/
        if (wclause->partitionClause != NIL)
                return false;

+       prosupport = get_func_support(wfunc->winfnoid);
+
+       /* Check if there's a support function for 'wfunc' */
+       if (!OidIsValid(prosupport))
+               return false;
+
        /* get the Expr from the other side of the OpExpr */
        if (wfunc_left)
                otherexpr = lsecond(opexpr->args);



--
Best Regards
Andy Fan (https://www.aliyun.com/)