On Sat, Jun 15, 2013 at 1:30 PM, Jeff Davis <pg...@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. Thanks, -- Hitoshi Harada
