|From:||Jeff Davis <pgsql(at)j-davis(dot)com>|
|To:||Nicholas White <n(dot)j(dot)white(at)gmail(dot)com>|
|Cc:||Bruce Momjian <bruce(at)momjian(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Troels Nielsen <bn(dot)troels(at)gmail(dot)com>, Hitoshi Harada <umi(dot)tanuki(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>|
|Subject:||Re: Request for Patch Feedback: Lag & Lead Window Functions Can Ignore Nulls|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
On Wed, 2014-04-16 at 12:50 +0100, Nicholas White wrote:
> Thanks for the detailed feedback, I'm sorry it took so long to
> incorporate it. I've attached the latest version of the patch, fixing
> in particular:
I took a good look at this today.
* It fails for offset of 0 with IGNORE NULLS. Fixed (trivial).
* The tests are locale-sensitive. Fixed (trivial).
* The leadlag_common function is just way too long. I refactored the
IGNORE NULLS code into it's own function (win_get_arg_ignore_nulls())
with the same API as WinGetFuncArgInPartition. This cleans things up
substantially, and makes it easier to add useful comments.
* "We're implementing the former semantics, so we'll need to correct
slightly" sounds arbitrary, but it's mandated by the standard. That
should be clarified.
* I did a lot of other refactoring within win_get_arg_ignore_nulls for
the constant case. I'm not done yet, and I'm not 100% sure it's a net
gain, because the code ended up a little longer. But the previous
version was quite hard to follow because of so many special cases around
positive versus negative offsets. For instance, having the negative
'next' value in your code actually means something quite different than
when it's positive, but it took me a while to figure that out, so I made
it into two variables. I hope my code is moving it in a direction that's
easier for others to understand.
Please let me know if you think I am making things worse with my
refactorings. Otherwise I'll keep working on this and hopefully get it
The attached patch is still a WIP; just posting it here in case you see
any red flags.
|Next Message||Rajeev rastogi||2014-07-07 04:20:12||Re: [RFC: bug fix?] Connection attempt block forever when the synchronous standby is not running|
|Previous Message||Fujii Masao||2014-07-07 03:50:17||Re: 9.4 documentation: duplicate paragraph in logical decoding example|