2010/2/13 Tom Lane <t...@sss.pgh.pa.us>: > Hitoshi Harada <umi.tan...@gmail.com> writes: >> [ more_frame_options patch ] > > Committed after rather extensive revisions.
Thanks a lot. > I'm not terribly happy with the changes you made in WinGetFuncArgInPartition > and WinGetFuncArgInFrame to force the window function mark to not go > past frame start in some modes. Not only is that pretty ugly, but I > think it can mask bugs in window functions: it's an error for a window > function to fetch a row before what it has set its mark to be, but in > some cases that wouldn't be detected because of this change. I think > it would be better to revert those changes and find another method of > protecting fetches needed to determine the frame head. One idea is > to create a separate read pointer that tracks the frame head whenever > actual fetches of the frame head might be needed by update_frameheadpos. > I committed it without changing that, but I think this should be > revisited before trying to add the RANGE value PRECEDING/FOLLOWING > options, because those will substantially expand the number of cases > where that hack affects the behavior. Well, you're right. In addition to this topic, I concern a little about changing row fetching in aggregate from spool_tuples() to window_gettupleslot(), for performance reason. It's needed to support extended frame options, but for original basic frame options it may get slow. Anyway, I agree to revisit and refactor to executor logic. Regards, -- Hitoshi Harada -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
