public inbox for [email protected]  
help / color / mirror / Atom feed
From: Zhihong Yu <[email protected]>
To: David Rowley <[email protected]>
Cc: Andy Fan <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: PostgreSQL Developers <[email protected]>
Subject: Re: Window Function "Run Conditions"
Date: Wed, 6 Apr 2022 20:45:56 -0700
Message-ID: <CALNJ-vTaG0i+Sx4bruXDKNnppfMJS0nt7E=7Roo7VXAuxZFWiw@mail.gmail.com> (raw)
In-Reply-To: <CAApHDvpou0r+AOT9JZZEsiANQY=w=BTiRXgWR230NzOG7D3VUg@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>
	<CAApHDvoe=MqH66pDWynUtdammYHiB8m+zE7LvL_CGuxm1yK4BQ@mail.gmail.com>
	<CAApHDvpYYndDJfGCmNA-o60VLb8D9OB1deh-qcsTiDjxVcFg_w@mail.gmail.com>
	<[email protected]>
	<CAApHDvp+rzxGh58hc3Fmr90PR-M=JOg96X0B5tfvY4pTKqNa3g@mail.gmail.com>
	<CAKU4AWoiqPd_DdOA_0PWZ_e9cV6e1yP6F0VmxaLjedwTWQtBJA@mail.gmail.com>
	<CAKU4AWpmDGMxQ3nJfj=awQAgcL-jP2p67MtpiHFZX77NaZ2hxQ@mail.gmail.com>
	<CAApHDvrU132PxjuLakFrQhjR_b40LHAqhCzk-q2iGOST+-6dCw@mail.gmail.com>
	<CAKU4AWrzzC4GCe2Mo_fYxc_SY=Ss1i0x-AEMjP_mxZYe6aMw=w@mail.gmail.com>
	<CAApHDvpou0r+AOT9JZZEsiANQY=w=BTiRXgWR230NzOG7D3VUg@mail.gmail.com>

On Wed, Apr 6, 2022 at 7:36 PM David Rowley <[email protected]> wrote:

> On Wed, 6 Apr 2022 at 00:59, Andy Fan <[email protected]> wrote:
> >
> > On Tue, Apr 5, 2022 at 7:49 PM David Rowley <[email protected]>
> wrote:
> >> Yeah, there is more performance to be had than even what you've done
> >> there.  There's no reason really for spool_tuples() to do
> >> tuplestore_puttupleslot() when we're not in run mode.
> >
> >
> > Yeah, this is a great idea.
>
> I've attached an updated patch that does most of what you mentioned.
> To make this work I had to add another state to the WindowAggStatus.
> This new state is what the top-level WindowAgg will move into when
> there's a PARTITION BY clause and the run condition becomes false.
> The new state is named WINDOWAGG_PASSTHROUGH_STRICT, which does all
> that WINDOWAGG_PASSTHROUGH does plus skips tuplestoring tuples during
> the spool.  We must still spool those tuples when we're not the
> top-level WindowAgg so that we can send those out to any calling
> WindowAgg nodes. They'll need those so they return the correct result.
>
> This means that for intermediate WindowAgg nodes, when the
> runcondition becomes false, we only skip evaluation of WindowFuncs.
> WindowAgg nodes above us cannot reference these, so there's no need to
> evaluate them, plus, if there's a run condition then these tuples will
> be filtered out in the final WindowAgg node.
>
> For the top-level WindowAgg node, when the run condition becomes false
> we can save quite a bit more work. If there's no PARTITION BY clause,
> then we're done. Just return NULL.  When there is a PARTITION BY
> clause we move into WINDOWAGG_PASSTHROUGH_STRICT which allows us to
> skip both the evaluation of WindowFuncs and also allows us to consume
> tuples from our outer plan until we get a tuple belonging to another
> partition.  No need to tuplestore these tuples as they're being
> filtered out.
>
> Since intermediate WindowAggs cannot filter tuples, all the filtering
> must occur in the top-level WindowAgg.  This cannot be done by way of
> the run condition as the run condition is special as when it becomes
> false, we don't check again to see if it's become true.  A sort node
> between the WindowAggs can change the tuple order (i.e previously
> monotonic values may no longer be monotonic) so it's only valid to
> evaluate the run condition that's meant for the WindowAgg node it was
> intended for.  To filter out the tuples that don't match the run
> condition from intermediate WindowAggs in the top-level WindowAgg,
> what I've done is introduced quals for WindowAgg nodes.  This means
> that we can now see Filter in EXPLAIN For WindowAgg and "Rows Removed
> by Filter".
>
> Why didn't I just do the filtering in the outer query like was
> happening before?  The problem is that when we push the quals down
> into the subquery, we don't yet have knowledge of which order that the
> WindowAggs will be evaluated in.  Only run conditions from
> intermediate WindowAggs will ever make it into the Filter, and we
> don't know which one the top-level WindowAgg will be until later in
> planning. To do the filtering in the outer query we'd need to push
> quals back out the subquery again. It seems to me to be easier and
> better to filter them out lower down in the plan.
>
> Since the top-level WindowAgg node can now filter tuples, the executor
> node had to be given a for(;;) loop so that it goes around again for
> another tuple after it filters a tuple out.
>
> I've also updated the commit message which I think I've made quite
> clear about what we optimise and how it's done.
>
> > And I would suggest the below fastpath for this feature.
> > -                               if
> (check_and_push_window_quals(subquery, rte, rti, clause))
> > +                               if (!subquery->hasWindowFuncs ||
> check_and_push_window_quals(subquery, rte, rti, clause))
>
> Good idea. Thanks!
>
> David
>
Hi,

+                * We must keep the original qual in place if there is a
+                * PARTITION BY clause as the top-level WindowAgg remains in
+                * pass-through mode and does nothing to filter out unwanted
+                * tuples.
+                */
+               *keep_original = false;

The comment talks about keeping original qual but the assignment uses the
value false.
Maybe the comment can be rephrased so that it matches the assignment.

Cheers


view thread (12+ 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], [email protected]
  Subject: Re: Window Function "Run Conditions"
  In-Reply-To: <CALNJ-vTaG0i+Sx4bruXDKNnppfMJS0nt7E=7Roo7VXAuxZFWiw@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