public inbox for [email protected]
help / color / mirror / Atom feedFrom: Zhihong Yu <[email protected]>
To: David Rowley <[email protected]>
Cc: Andy Fan <[email protected]>
Cc: PostgreSQL Developers <[email protected]>
Subject: Re: Window Function "Run Conditions"
Date: Tue, 22 Mar 2022 16:54:08 -0700
Message-ID: <CALNJ-vTG+SDaCs_u0G9TDUngT7zVGcPxaYAJf+cjbbh8G_3GXA@mail.gmail.com> (raw)
In-Reply-To: <CAApHDvqtwE4bZxAdk+LQgVkKM+Em-sHybON_OX4YbFs+sQUKWg@mail.gmail.com>
References: <CAApHDvqvp3At8++yF8ij06sdcoo1S_b2YoaT9D4Nf+MObzsrLQ@mail.gmail.com>
<CAApHDvrxxkc9dgoRJUHy6U+XmGjSXRF=fQ-_U1-EvzTjCA9XKw@mail.gmail.com>
<CALNJ-vSRgdOsMx-oiOZcMJzcxoRCo0nJr=U8us4eJR8R6n9ZtA@mail.gmail.com>
<CAApHDvpiAUmUMTETAua8XCLFJiFhsZkCwuiW50KDvehexA-SLA@mail.gmail.com>
<CAKU4AWq5MB_inrYUwPQYXU1fCopCwgYK53N2uYGYFBH3QEhZMA@mail.gmail.com>
<CAApHDvp8PSY3GJXNzzrBPJtzRgQ-C85OPy-bypFGmwaT3AjCjQ@mail.gmail.com>
<CAKU4AWrWzXFuGW4d3YTu=TJSWQKTB=J3E8S_5zyuLNR7npomAA@mail.gmail.com>
<CAApHDvqtwE4bZxAdk+LQgVkKM+Em-sHybON_OX4YbFs+sQUKWg@mail.gmail.com>
On Tue, Mar 22, 2022 at 3:24 PM David Rowley <[email protected]> wrote:
> On Thu, 26 Aug 2021 at 14:54, Andy Fan <[email protected]> wrote:
> >
> > On Thu, Aug 19, 2021 at 2:35 PM David Rowley <[email protected]>
> wrote:
> > >
> > > On Thu, 19 Aug 2021 at 00:20, Andy Fan <[email protected]>
> 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.
>
> Thanks for looking at that.
>
> My current thoughts are that it just feels a little too risky to
> adjust the comparison function that sorts the window clauses to pay
> attention to the run-condition.
>
> We would need to ensure that there's just a single window function
> with a run condition as it wouldn't be valid for there to be multiple.
> It would be easy enough to ensure we only push quals into just 1
> window clause, but that and meddling with the evaluation order has
> trade-offs. To do that properly, we'd likely want to consider the
> costs when deciding which window clause would benefit from having
> quals pushed the most. Plus, if we start messing with the evaluation
> order then we'd likely really want some sort of costing to check if
> pushing a qual and adjusting the evaluation order is actually cheaper
> than not pushing the qual and keeping the clauses in the order that
> requires the minimum number of sorts. The planner is not really
> geared up for costing things like that properly, that's why we just
> assume the order with the least sorts is best. In reality that's often
> not going to be true as an index may exist and we might want to
> evaluate a clause first if we could get rid of a sort and index scan
> instead. Sorting the window clauses based on their SortGroupClause is
> just the best we can do for now at that stage in planning.
>
> I think it's safer to just disable the optimisation when there are
> multiple window clauses. Multiple matching clauses are merged
> already, so it's perfectly valid to have multiple window functions,
> it's just they must share the same window clause. I don't think
> that's terrible as with the major use case that I have in mind for
> this, the window function is only added to limit the number of rows.
> In most cases I can imagine, there'd be no reason to have an
> additional window function with different frame options.
>
> I've attached an updated patch.
>
Hi,
The following code seems to be common between if / else blocks (w.r.t.
wfunc_left) of find_window_run_conditions().
+ op = get_opfamily_member(opinfo->opfamily_id,
+ opinfo->oplefttype,
+ opinfo->oprighttype,
+ newstrategy);
+
+ newopexpr = (OpExpr *) make_opclause(op,
+ opexpr->opresulttype,
+ opexpr->opretset,
+ otherexpr,
+ (Expr *) wfunc,
+ opexpr->opcollid,
+ opexpr->inputcollid);
+ newopexpr->opfuncid = get_opcode(op);
+
+ *keep_original = true;
+ runopexpr = newopexpr;
It would be nice if this code can be shared.
+ WindowClause *wclause = (WindowClause *)
+ list_nth(subquery->windowClause,
+ wfunc->winref - 1);
The code would be more readable if list_nth() is indented.
+ /* Check the left side of the OpExpr */
It seems the code for checking left / right is the same. It would be better
to extract and reuse the code.
Cheers
view thread (16+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Window Function "Run Conditions"
In-Reply-To: <CALNJ-vTG+SDaCs_u0G9TDUngT7zVGcPxaYAJf+cjbbh8G_3GXA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox