MIME-Version: 1.0
References: 
 <CAApHDvqvp3At8++yF8ij06sdcoo1S_b2YoaT9D4Nf+MObzsrLQ@mail.gmail.com>
 <CAApHDvrxxkc9dgoRJUHy6U+XmGjSXRF=fQ-_U1-EvzTjCA9XKw@mail.gmail.com>
In-Reply-To: 
 <CAApHDvrxxkc9dgoRJUHy6U+XmGjSXRF=fQ-_U1-EvzTjCA9XKw@mail.gmail.com>
From: Zhihong Yu <zyu@yugabyte.com>
Date: Mon, 16 Aug 2021 08:57:12 -0700
Message-ID: 
 <CALNJ-vSRgdOsMx-oiOZcMJzcxoRCo0nJr=U8us4eJR8R6n9ZtA@mail.gmail.com>
Subject: Re: Window Function "Run Conditions"
To: David Rowley <dgrowleyml@gmail.com>
Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/alternative; boundary="000000000000efe26c05c9af2ce9"
Archived-At: 
 <https://www.postgresql.org/message-id/CALNJ-vSRgdOsMx-oiOZcMJzcxoRCo0nJr%3DU8us4eJR8R6n9ZtA%40mail.gmail.com>
Precedence: bulk

--000000000000efe26c05c9af2ce9
Content-Type: text/plain; charset="UTF-8"

On Mon, Aug 16, 2021 at 3:28 AM David Rowley <dgrowleyml@gmail.com> wrote:

> On Thu, 1 Jul 2021 at 21:11, David Rowley <dgrowleyml@gmail.com> wrote:
> > 1) Unsure of the API to the prosupport function.  I wonder if the
> > prosupport function should just be able to say if the function is
> > either monotonically increasing or decreasing or neither then have
> > core code build a qual.  That would make the job of building new
> > functions easier, but massively reduce the flexibility of the feature.
> > I'm just not sure it needs to do more in the future.
>
> I looked at this patch again today and ended up changing the API that
> I'd done for the prosupport functions.  These just now set a new
> "monotonic" field in the (also newly renamed)
> SupportRequestWFuncMonotonic struct. This can be set to one of the
> values from the newly added MonotonicFunction enum, namely:
> MONOTONICFUNC_NONE, MONOTONICFUNC_INCREASING, MONOTONICFUNC_DECREASING
> or MONOTONICFUNC_BOTH.
>
> I also added handling for a few more cases that are perhaps rare but
> could be done with just a few lines of code. For example; COUNT(*)
> OVER() is MONOTONICFUNC_BOTH as it can neither increase nor decrease
> for a given window partition. I think technically all of the standard
> set of aggregate functions could have a prosupport function to handle
> that case. Min() and Max() could go a little further, but I'm not sure
> if adding handling for that would be worth it, and if someone does
> think that it is worth it, then I'd rather do that as a separate
> patch.
>
> I put the MonotonicFunction enum in plannodes.h. There's nothing
> specific about window functions or support functions. It could, for
> example, be reused again for something else such as monotonic
> set-returning functions.
>
> One thing which I'm still not sure about is where
> find_window_run_conditions() should be located. Currently, it's in
> allpaths.c but that does not really feel like the right place to me.
> We do have planagg.c in src/backend/optimizer/plan, maybe we need
> planwindow.c?
>
> David
>
Hi,

+               if ((res->monotonic & MONOTONICFUNC_INCREASING) ==
MONOTONICFUNC_INCREASING)

The above can be simplified as 'if (res->monotonic &
MONOTONICFUNC_INCREASING) '

Cheers

--000000000000efe26c05c9af2ce9
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote">=
<div dir=3D"ltr" class=3D"gmail_attr">On Mon, Aug 16, 2021 at 3:28 AM David=
 Rowley &lt;<a href=3D"mailto:dgrowleyml@gmail.com">dgrowleyml@gmail.com</a=
>&gt; wrote:<br></div><blockquote class=3D"gmail_quote" style=3D"margin:0px=
 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On =
Thu, 1 Jul 2021 at 21:11, David Rowley &lt;<a href=3D"mailto:dgrowleyml@gma=
il.com" target=3D"_blank">dgrowleyml@gmail.com</a>&gt; wrote:<br>
&gt; 1) Unsure of the API to the prosupport function.=C2=A0 I wonder if the=
<br>
&gt; prosupport function should just be able to say if the function is<br>
&gt; either monotonically increasing or decreasing or neither then have<br>
&gt; core code build a qual.=C2=A0 That would make the job of building new<=
br>
&gt; functions easier, but massively reduce the flexibility of the feature.=
<br>
&gt; I&#39;m just not sure it needs to do more in the future.<br>
<br>
I looked at this patch again today and ended up changing the API that<br>
I&#39;d done for the prosupport functions.=C2=A0 These just now set a new<b=
r>
&quot;monotonic&quot; field in the (also newly renamed)<br>
SupportRequestWFuncMonotonic struct. This can be set to one of the<br>
values from the newly added MonotonicFunction enum, namely:<br>
MONOTONICFUNC_NONE, MONOTONICFUNC_INCREASING, MONOTONICFUNC_DECREASING<br>
or MONOTONICFUNC_BOTH.<br>
<br>
I also added handling for a few more cases that are perhaps rare but<br>
could be done with just a few lines of code. For example; COUNT(*)<br>
OVER() is MONOTONICFUNC_BOTH as it can neither increase nor decrease<br>
for a given window partition. I think technically all of the standard<br>
set of aggregate functions could have a prosupport function to handle<br>
that case. Min() and Max() could go a little further, but I&#39;m not sure<=
br>
if adding handling for that would be worth it, and if someone does<br>
think that it is worth it, then I&#39;d rather do that as a separate<br>
patch.<br>
<br>
I put the MonotonicFunction enum in plannodes.h. There&#39;s nothing<br>
specific about window functions or support functions. It could, for<br>
example, be reused again for something else such as monotonic<br>
set-returning functions.<br>
<br>
One thing which I&#39;m still not sure about is where<br>
find_window_run_conditions() should be located. Currently, it&#39;s in<br>
allpaths.c but that does not really feel like the right place to me.<br>
We do have planagg.c in src/backend/optimizer/plan, maybe we need<br>
planwindow.c?<br>
<br>
David<br></blockquote><div>Hi,</div><div><br></div><div>+ =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if ((res-&gt;monotonic &amp; MONOTONICFUNC_=
INCREASING) =3D=3D MONOTONICFUNC_INCREASING)</div><div><br></div><div>The a=
bove can be simplified as &#39;if (res-&gt;monotonic &amp; MONOTONICFUNC_IN=
CREASING) &#39;</div><div><br></div><div>Cheers</div></div></div>

--000000000000efe26c05c9af2ce9--