From: | Jeff Davis <pgsql(at)j-davis(dot)com> |
---|---|
To: | Nicholas White <n(dot)j(dot)white(at)gmail(dot)com> |
Cc: | Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>, Troels Nielsen <bn(dot)troels(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(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 |
Date: | 2013-07-05 22:34:49 |
Message-ID: | 1373063689.7056.44.camel@jdavis |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, 2013-07-01 at 18:20 -0400, Nicholas White wrote:
> > 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-nulls
> flag) I create a child of the WindowDef and override the frameOptions.
> This has the useful side-effect of making pg_get_viewdef work as
> expected (the previous iteration of the patch produced a copy of the
> window definintion, not the window name, as it was using a nameless
> clone), although the output has parentheses around the view name:
>
A couple comments:
* We shouldn't create an arbitrary number of duplicate windows when
many aggregates are specified with IGNORE NULLS.
* It's bad form to modify a list while iterating through it. This is
just a style issue because there's a break afterward, anyway.
Also, I'm concerned that we're changing a reference of the form:
OVER w
into:
OVER (w)
in a user-visible way. Is there a problem with having two windowdefs in
the p_windowdefs list with the same name and different frameOptions?
I think you could just change the matching criteria to be a matching
name and matching frameOptions. In the loop, if you find a matching name
but frameOptions doesn't match, keep a pointer to the windowdef and
create a new one at the end of the loop with the same name.
You'll have to be a little careful that any other code knows that names
can be duplicated in the list though.
Regards,
Jeff Davis
From | Date | Subject | |
---|---|---|---|
Next Message | Noah Misch | 2013-07-05 22:39:11 | Re: Preventing tuple-table leakage in plpgsql |
Previous Message | Jeff Davis | 2013-07-05 21:36:10 | Re: Request for Patch Feedback: Lag & Lead Window Functions Can Ignore Nulls |