From: | "Hitoshi Harada" <umi(dot)tanuki(at)gmail(dot)com> |
---|---|
To: | "David Rowley" <dgrowley(at)gmail(dot)com> |
Cc: | pgsql-hackers(at)postgresql(dot)org, heikki(dot)linnakangas(at)enterprisedb(dot)com |
Subject: | Re: Windowing Function Patch Review -> Standard Conformance |
Date: | 2008-11-24 07:48:55 |
Message-ID: | e08cc0400811232348qa31d939x9b1c0c5aa34e66b3@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
patch-2
2008/11/24 Hitoshi Harada <umi(dot)tanuki(at)gmail(dot)com>:
> 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
>
--
Hitoshi Harada
Attachment | Content-Type | Size |
---|---|---|
window_functions.patch.20081124-2.gz | application/x-gzip | 92.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tao Ma | 2008-11-24 07:57:20 | huge query tree cost too much time to copyObject() |
Previous Message | Hitoshi Harada | 2008-11-24 07:48:24 | Re: Windowing Function Patch Review -> Standard Conformance |