On Sat, Jun 15, 2013 at 1:30 PM, Jeff Davis <pgsql@j-davis.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.