From: | Hitoshi Harada <umi(dot)tanuki(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: review: More frame options in window functions |
Date: | 2010-02-13 06:42:05 |
Message-ID: | e08cc0401002122242w5995fefanc485bc57eabdfece@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
2010/2/13 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> Hitoshi Harada <umi(dot)tanuki(at)gmail(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Greg Smith | 2010-02-13 08:34:23 | Re: Re: [COMMITTERS] pgsql: Remove old-style VACUUM FULL (which was known for a little while |
Previous Message | Alex Hunsaker | 2010-02-13 04:14:07 | Re: Package namespace and Safe init cleanup for plperl [PATCH] |