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

2014-07-21 Thread Jeff Davis
On Thu, 2014-07-10 at 23:43 -0700, Jeff Davis wrote: > On Mon, 2014-07-07 at 01:21 -0700, Jeff Davis wrote: > > On Sun, 2014-07-06 at 21:11 -0700, Jeff Davis wrote: > > > 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

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

2014-07-10 Thread Jeff Davis
On Mon, 2014-07-07 at 01:21 -0700, Jeff Davis wrote: > On Sun, 2014-07-06 at 21:11 -0700, Jeff Davis wrote: > > 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

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

2014-07-07 Thread Jeff Davis
On Sun, 2014-07-06 at 21:11 -0700, Jeff Davis wrote: > 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: Looking a little more: *

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

2014-07-06 Thread Jeff Davis
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

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

2014-04-16 Thread Nicholas White
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: > We have this block: I've re-written this so it only does a single pass through the window definitions (my patch originally added a second pass), and

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

2013-11-29 Thread Bruce Momjian
On Fri, Nov 29, 2013 at 01:21:27PM -0800, Jeff Davis wrote: > On Fri, 2013-11-29 at 16:18 -0500, Bruce Momjian wrote: > > On Fri, Jul 5, 2013 at 02:36:10PM -0700, Jeff Davis wrote: > > > On Fri, 2013-06-28 at 13:26 -0400, Robert Haas wrote: > > > > I haven't really reviewed the windowing-related c

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

2013-11-29 Thread Jeff Davis
On Fri, 2013-11-29 at 16:18 -0500, Bruce Momjian wrote: > On Fri, Jul 5, 2013 at 02:36:10PM -0700, Jeff Davis wrote: > > On Fri, 2013-06-28 at 13:26 -0400, Robert Haas wrote: > > > I haven't really reviewed the windowing-related code in depth; I > > > thought Jeff might jump back in for that part

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

2013-11-29 Thread Bruce Momjian
On Fri, Jul 5, 2013 at 02:36:10PM -0700, Jeff Davis wrote: > On Fri, 2013-06-28 at 13:26 -0400, Robert Haas wrote: > > 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? > >

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

2013-07-05 Thread Jeff Davis
On Fri, 2013-06-28 at 13:26 -0400, Robert Haas wrote: > 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? Yes, getting back into this patch now after a bit of delay. Regards,

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

2013-07-05 Thread Jeff Davis
On Mon, 2013-07-01 at 07:40 -0400, Robert Haas wrote: > On Sun, Jun 30, 2013 at 10:07 PM, Nicholas White wrote: > > I've attached another iteration of the patch that fixes the multiple-window > > bug and adds (& uses) a function to create a Bitmapset using a custom > > allocator. I don't think the

[HACKERS] Request for Patch Feedback: Lag & Lead Window Functions Can Ignore Nulls

2013-07-01 Thread Nicholas White
> pg_get_viewdef() needs to be updated Ah, good catch - I've fixed this in the attached. I also discovered that there's a parent-child hierarchy of WindowDefs (using relname->name), so instead of cloning the WindowDef (in parse_agg.c) if the frameOptions are different (e.g. by adding the ignore-nu

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

2013-07-01 Thread Dean Rasheed
On 1 July 2013 03:07, Nicholas White wrote: > I've attached another iteration of the patch that fixes the multiple-window > bug and adds (& uses) a function to create a Bitmapset using a custom > allocator. I don't think there's any outstanding problems with it now. > I just realised there is ano

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

2013-07-01 Thread Robert Haas
On Sun, Jun 30, 2013 at 10:07 PM, Nicholas White wrote: > I've attached another iteration of the patch that fixes the multiple-window > bug and adds (& uses) a function to create a Bitmapset using a custom > allocator. I don't think there's any outstanding problems with it now. I think the right

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

2013-06-30 Thread Dean Rasheed
On 1 July 2013 03:07, Nicholas White wrote: >> Alternatively, it might be trivial to make all aggregate functions work >> with ignore nulls in a window context > > This is a good idea, but I'd like to keep the scope of this patch limited > for the time being Agreed. > - I'll look at doing this

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

2013-06-30 Thread Nicholas White
I've attached another iteration of the patch that fixes the multiple-window bug and adds (& uses) a function to create a Bitmapset using a custom allocator. I don't think there's any outstanding problems with it now. > Alternatively, it might be trivial to make all aggregate functions work with ig

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

2013-06-30 Thread Nicholas White
> this should throw a FEATURE_NOT_SUPPORTED error if it is used for window functions that don't support it > arbitrary aggregate functions over a window ... should also throw a FEATURE_NOT_SUPPORTED error. Fixed (with test cases) in the attached patch. > because the same window may be shared by m

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

2013-06-30 Thread Dean Rasheed
On 29 June 2013 17:30, Jeff Davis wrote: > > On Mon, 2013-06-24 at 18:01 +0100, Nicholas White wrote: >> Good catch - I've attached a patch to address your point 1. It now >> returns the below (i.e. correctly doesn't fill in the saved value if >> the index is out of the window. However, I'm not su

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

2013-06-29 Thread Jeff Davis
On Mon, 2013-06-24 at 18:01 +0100, Nicholas White wrote: > Good catch - I've attached a patch to address your point 1. It now > returns the below (i.e. correctly doesn't fill in the saved value if > the index is out of the window. However, I'm not sure whether (e.g.) > lead-2-ignore-nulls means co

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

2013-06-28 Thread Nicholas White
I've fixed the problems you mentioned (see attached) - sorry, I was a bit careless with the docs. > + null_values = (Bitmapset *) WinGetPartitionLocalMemory( > + winobj, > + BITMAPSET_SIZE(words_needed)); > + Assert(null_valu

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

2013-06-28 Thread Robert Haas
On Fri, Jun 28, 2013 at 12:29 PM, Nicholas White 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. default are evaluated wit

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

2013-06-28 Thread Nicholas White
> This patch will need to be rebased over those changes See attached - lead-lag-ignore-nulls.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers

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

2013-06-28 Thread Robert Haas
On Fri, Jun 28, 2013 at 11:41 AM, Alvaro Herrera wrote: > Robert Haas escribió: >> On Thu, Jun 27, 2013 at 8:52 PM, Nicholas White wrote: > >> The documentation you've added reads kind of funny to me: >> >> + [respect nulls]|[ignore nulls] >> >> Wouldn't we normally write that as [ [ RESP

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

2013-06-28 Thread Alvaro Herrera
Robert Haas escribió: > On Thu, Jun 27, 2013 at 8:52 PM, Nicholas White wrote: > The documentation you've added reads kind of funny to me: > > + [respect nulls]|[ignore nulls] > > Wouldn't we normally write that as [ [ RESPECT | IGNORE ] NULLS ] ? I think it should be [ { RESPECT | IGN

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

2013-06-28 Thread Robert Haas
On Thu, Jun 27, 2013 at 8:52 PM, Nicholas White wrote: >> The result of the current patch using lead > > Actually, I think I agree with you (and, FWIW, so does Oracle: > http://docs.oracle.com/cd/E11882_01/server.112/e25554/analysis.htm#autoId18). > I've refactored the window function's implementa

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

2013-06-27 Thread Nicholas White
> The result of the current patch using lead Actually, I think I agree with you (and, FWIW, so does Oracle: http://docs.oracle.com/cd/E11882_01/server.112/e25554/analysis.htm#autoId18). I've refactored the window function's implementation so that (e.g.) lead(5) means the 5th non-null value away in

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

2013-06-24 Thread Nicholas White
OK - I've attached another iteration of the patch with Troels' grammar changes. I think the only issue remaining is what the standard says about lead semantics. Thanks - lead-lag-ignore-nulls.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To

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

2013-06-24 Thread Robert Haas
On Fri, Jun 21, 2013 at 6:29 PM, Troels Nielsen wrote: > The grammar conflict appears to be because of ambiguities in: > 1. table_ref (used exclusively in FROM clauses) > 2. index_elem (used exclusively in INDEX creation statements). > > Now, this doesn't seem to make much sense, as AFAICT window

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

2013-06-24 Thread Nicholas White
Good catch - I've attached a patch to address your point 1. It now returns the below (i.e. correctly doesn't fill in the saved value if the index is out of the window. However, I'm not sure whether (e.g.) lead-2-ignore-nulls means count forwards two rows, and if that's null use the last one you've

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

2013-06-21 Thread Troels Nielsen
Hello all I've been examining PostgreSQL to gain a greater understanding of RDBMS. (Thanks for a nice, very educational system!) In the process I've been looking into a few problems and the complications of this patch appeared relatively uninvolved, so I tried to look for a solution. I found the

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

2013-06-21 Thread Robert Haas
On Fri, Jun 21, 2013 at 11:33 AM, Jeff Davis wrote: >> Regardless of what syntax we settle on, we should also make sure that >> the conflict is intrinsic to the grammar and can't be factored out, as >> Tom suggested upthread. It's not obvious to me what the actual >> ambiguity is here. If you've

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

2013-06-21 Thread Jeff Davis
On Fri, 2013-06-21 at 09:21 -0400, Robert Haas wrote: > The other question here is - do we actually have the grammar right? > As in, is this actually the syntax we're supposed to be implementing? > It looks different from what's shown here, where the IGNORE NULLS is > inside the function's parenthe

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

2013-06-21 Thread Robert Haas
On Fri, Jun 21, 2013 at 12:18 AM, Jeff Davis wrote: > On Thu, 2013-06-20 at 10:03 -0400, Robert Haas wrote: >> I think the question is whether this feature is really worth adding >> new reserved keywords for. I have a hard time saying we shouldn't >> support something that's part of the SQL stand

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

2013-06-20 Thread Jeff Davis
On Thu, 2013-06-20 at 10:03 -0400, Robert Haas wrote: > I think the question is whether this feature is really worth adding > new reserved keywords for. I have a hard time saying we shouldn't > support something that's part of the SQL standard, but personally, > it's not something I've seen come u

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

2013-06-20 Thread Robert Haas
On Tue, Jun 18, 2013 at 6:27 PM, Nicholas White wrote: > Thanks for the reviews. I've attached a revised patch that has the lexer > refactoring Alvaro mentions (arbitarily using a goto rather than a bool > flag) and uses Jeff's idea of just storing the index of the last non-null > value (if there

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

2013-06-18 Thread Nicholas White
Thanks for the reviews. I've attached a revised patch that has the lexer refactoring Alvaro mentions (arbitarily using a goto rather than a bool flag) and uses Jeff's idea of just storing the index of the last non-null value (if there is one) in the window function's context (and not the Datum itse

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

2013-06-18 Thread Hitoshi Harada
On Sat, Jun 15, 2013 at 1:30 PM, Jeff Davis 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 buildin

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

2013-06-17 Thread Robert Haas
On Sat, Jun 15, 2013 at 9:37 PM, Alvaro Herrera wrote: > Nicholas White escribió: > >> For the parsing changes, it seems I can either make RESPECT and IGNORE >> reserved keywords, or add a lookahead to construct synthetic RESPECT NULLS >> and IGNORE NULLS keywords. The grammar wouldn't compile if

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

2013-06-15 Thread Alvaro Herrera
Nicholas White escribió: > For the parsing changes, it seems I can either make RESPECT and IGNORE > reserved keywords, or add a lookahead to construct synthetic RESPECT NULLS > and IGNORE NULLS keywords. The grammar wouldn't compile if RESPECT and > IGNORE were just normal unreserved keywords due

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

2013-06-15 Thread Jeff Davis
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

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

2013-03-24 Thread Nicholas White
Thanks for the feedback. For the parsing changes, it seems I can either make RESPECT and IGNORE reserved keywords, or add a lookahead to construct synthetic RESPECT NULLS and IGNORE NULLS keywords. The grammar wouldn't compile if RESPECT and IGNORE were just normal unreserved keywords due to ambig

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

2013-03-24 Thread Hitoshi Harada
On Sat, Mar 23, 2013 at 3:25 PM, Nicholas White wrote: > Thanks - I've added it here: > https://commitfest.postgresql.org/action/patch_view?id=1096 . > > I've also attached a revised version that makes IGNORE and RESPECT > UNRESERVED keywords (following the pattern of NULLS_FIRST and NULLS_LAST).

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

2013-03-23 Thread Nicholas White
Thanks - I've added it here: https://commitfest.postgresql.org/action/patch_view?id=1096 . I've also attached a revised version that makes IGNORE and RESPECT UNRESERVED keywords (following the pattern of NULLS_FIRST and NULLS_LAST). Nick On 23 March 2013 14:34, Tom Lane wrote: > Nicholas Whit

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

2013-03-23 Thread Tom Lane
Nicholas White writes: > The SQL standard defines a RESPECT NULLS or IGNORE NULLS option for lead, > lag, [...]. This is not implemented in PostgreSQL > (http://www.postgresql.org/docs/devel/static/functions-window.html) > I've had a go at implementing this, and I've attached the resulting patch.

[HACKERS] Request for Patch Feedback: Lag & Lead Window Functions Can Ignore Nulls

2013-03-23 Thread Nicholas White
> The SQL standard defines a RESPECT NULLS or IGNORE NULLS option for lead, lag, [...]. This is not implemented in PostgreSQL (http://www.postgresql.org/docs/devel/static/functions-window.html) I've had a go at implementing this, and I've attached the resulting patch. It's not finished yet, but I w

[HACKERS] Request for Patch Feedback: Lag & Lead Window Functions Can Ignore Nulls

2013-03-21 Thread Nicholas White
> The SQL standard defines a RESPECT NULLS or IGNORE NULLS option for lead, lag, [...]. This is not implemented in PostgreSQL (http://www.postgresql.org/docs/devel/static/functions-window.html) I've had a go at implementing this, and I've attached the resulting patch. It's not finished yet (there'