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

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.

Alternatively, you might look into setting a mark when you get a
non-NULL value. Then you could just always fetch the oldest one.
Unfortunately, I think that only works with const_offset=true... so the
previous idea might be better.

I'll leave it to someone else to review the grammar changes.

Regards,
Jeff Davis