Re: Request for Patch Feedback: Lag & Lead Window Functions Can Ignore Nulls

On Sat, Jun 15, 2013 at 1:30 PM, Jeff Davis <pgsql(at)j-davis(dot)com> wrote:

> On Sun, 2013-03-24 at 20:15 -0400, Nicholas White wrote:
>
> > I've redone the leadlag function changes to use datumCopy as you
> > suggested. However, I've had to remove the NOT_USED #ifdef around
> > datumFree so I can use it to avoid building up large numbers of copies
> > of Datums in the memory context while a query is executing. I've
> > attached the revised patch...
> >
> Comments:
>
> WinGetResultDatumCopy() calls datumCopy, which will just copy in the
> current memory context. I think you want it in the per-partition memory
> context, otherwise the last value in each partition will stick around
> until the query is done (so many partitions could be a problem). That
> should be easy enough to do by switching to the
> winobj->winstate->partcontext memory context before calling datumCopy,
> and then switching back.
>
> For that matter, why store the datum again at all? You can just store
> the offset of the last non-NULL value in that partition, and then fetch
> it using WinGetFuncArgInPartition(), right? We really want to avoid any
> per-tuple allocations.
>
>
I believe WinGetFuncArgInPartition is a bit slow if the offset is far from
the current row. So it might make sense to store the last-seen value, but
I'm not sure if we need to copy datum every time. I haven't looked into
the new patch.

Thanks,
--
Hitoshi Harada