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