Re: Windowing Function Patch Review -> Standard Conformance

patch-2

2008/11/24 Hitoshi Harada <umi.tanuki@gmail.com>:
> 2008/11/20 David Rowley <dgrowley@gmail.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
>



--
Hitoshi Harada