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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers