On Fri, Jun 28, 2013 at 12:29 PM, Nicholas White <n.j.white@gmail.com> wrote:
>> This patch will need to be rebased over those changes
>
> See attached -
Thanks. But I see a few issues...
+ [respect nulls]|[ignore nulls]
You fixed one of these but missed the other.
<replaceable class="parameter">default</replaceable> are evaluated with respect to the current row. If
omitted, <replaceable class="parameter">offset</replaceable> defaults to 1 and
- <replaceable class="parameter">default</replaceable> to null
+ <literal>IGNORE NULLS</> is specified then the function will
be evaluated
+ as if the rows containing nulls didn't exist. </entry> </row>
This looks like you dropped a line during cut-and-paste.
+ null_values = (Bitmapset *) WinGetPartitionLocalMemory(
+ winobj,
+ BITMAPSET_SIZE(words_needed));
+ Assert(null_values);
This certainly seems ugly - isn't there a way to accomplish this
without having to violate the Bitmapset abstraction?
+ * A slight edge case. Consider:
+ *
+ * A | lag(A, 1)
+ * 1 |
+ * 2 | 1
+ * | ?
pgindent will reformat this into oblivion. Use ----- markers around
the comment block as done elsewhere in the code where this is an
issue, or don't use ASCII art.
I haven't really reviewed the windowing-related code in depth; I
thought Jeff might jump back in for that part of it. Jeff, is that
something you're planning to do?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company