From: | Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> |
---|---|
To: | Hitoshi Harada <umi(dot)tanuki(at)gmail(dot)com> |
Cc: | David Rowley <dgrowley(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Windowing Function Patch Review -> Standard Conformance |
Date: | 2008-11-24 11:21:47 |
Message-ID: | 492A8E4B.4050409@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hitoshi Harada wrote:
> 2008/11/20 David Rowley <dgrowley(at)gmail(dot)com>:
>> 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.
Thanks! Here's what I've got this far I merged your latest patch into
this, as well as latest changes from CVS HEAD.
It's a bit of a mess, but here's the big changes I've done:
- Removed all the iterator stuff. You can just call
WinGetPart/FrameGetArg repeatedly. It ought to be just as fast if done
right in nodeWindow.c.
- Removed all the Shrinked/Extended stuff, as it's not needed until we
have full support for window frames.
- Removed all the WinRow* functions, you can just call WinFrame/PartGet*
functions, using WINDOW_SEEK_CURRENT
- Removed WinFrame/PartGetTuple functions. They were only used for
determining if two tuples are peer with each other, so I added a
WinRowIsPeer function instead.
- Removed all the buffering strategy stuff. Currently, the whole
partition is always materialized. That's of course not optimal; I'm
still thinking we should just read the tuples from the outer node
lazily, on-demand, instead. To avoid keeping all tuples in the partition
in tuplestore, when not needed, should add an extra function to trim the
tuplestore.
There's now a lot less code in nodeWindow.c, and I'm starting to
understand most of what-s left :-).
TODO
- clean up the comments and other mess.
- Modify WinPart/FrameGetArg so that it's passed the parameter number
instead of an Expr node.
I'll continue working on the executor, but please let me know what you
think.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2008-11-24 12:18:19 | Re: Autoconf, libpq and replacement function |
Previous Message | Pavel Stehule | 2008-11-24 11:16:33 | Re: [Review] Grouping Sets patch |