Re: Windowing Function Patch Review -> Standard Conformance

2008/11/20 David Rowley <dgrowley(at)gmail(dot)com>:
> -- The following query gives incorrect results on the
> -- maxhighbid column
>
> SELECT auctionid,
> category,
> description,
> highestbid,
> reserve,
> MAX(highestbid) OVER (ORDER BY auctionid) AS maxhighbid,
> MAX(reserve) OVER() AS highest_reserve
> FROM auction_items;
>
> If you remove the highest_reserve column you get the correct results.
>
> The bug also affects MIN, AVG, COUNT, STDDEV but not SUM.
Good report! I fixed this bug, which was by a trival missuse of
list_concat() in the planner. This was occurred when the first
aggregate trans func is strict and the second aggregate argument may
be null. Yep, the argument of the second was implicitly passed to the
first wrongly. That's why it didn't occur if you delete the second
MAX().

I added a test case with the existing data emulating yours (named
"strict aggs") but if it is wrong, let me know.

>
> I've also had a little look at the documentation included in the patch.
> At first scan the only thing I can see that is wrong is
>
> +
> + <para>
> + Window functions are put in the <command>SELECT</command> list.
> + It is forbidden anywhere else such as <literal>GROUP BY</literal>,
>
> I think this needs to be rewritten as they are allowed in the ORDER BY
> clause, works in patch and spec says the same.
>
> Maybe it should read something more like:
>
> "Window functions are only permitted in the <command>SELECT</command> and
> the <command>ORDER BY</command> clause of the query. They are forbidden
> anywhere else..." etc.
You're right. I modified this part, with <literal> tag instead of
<command>. I am not sure about the clear difference of <command> and
<literal> but followed what the surroundings tell.

> I won't be around until Monday evening (Tuesday morning JST). I'll pickup
> the review again there. It's really time for me to push on with this review.
> The patch is getting closer to getting the thumbs up from me. I'm really
> hoping that can happen on Monday. Then it's over to Heikki for more code
> feedback.

This time I only fixed some bugs and rebased against the HEAD, but did
not refactored. I can figure out some part of what Heikki claimed but
not all, so I'd like to see what he will send and post another patch
if needed.

Regards,

--
Hitoshi Harada