On Thu, Dec 11, 2014 at 3:36 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: > I don't really have any comments on the algorithms yet, having spent too > much time trying to figure out underdocumented data structures to get to > the algorithms. However, noting the addition of list_intersection_int() > made me wonder whether you'd not be better off reducing the integer lists > to bitmapsets a lot sooner, perhaps even at parse analysis. > list_intersection_int() is going to be O(N^2) by nature. Maybe N can't > get large enough to matter in this context, but I do see places that > seem to be concerned about performance. > > I've not spent any real effort looking at gsp2.patch yet, but it seems > even worse off comment-wise: if there's any explanation in there at all > of what a "chained aggregate" is, I didn't find it. I'd also counsel you > to find some other way to do it than putting bool chain_head fields in > Aggref nodes; that looks like a mess, eg, it will break equal() tests > for expression nodes that probably should still be seen as equal. > > I took a quick look at gsp-u.patch. It seems like that approach should > work, with of course the caveat that using CUBE/ROLLUP as function names > in a GROUP BY list would be problematic. I'm not convinced by the > commentary in ruleutils.c suggesting that extra parentheses would help > disambiguate: aren't extra parentheses still going to contain grouping > specs according to the standard? Forcibly schema-qualifying such function > names seems like a less fragile answer on that end. Based on those comments, I am marking this patch as "Returned with Feedback" on the CF app for 2014-10. Andrew, feel free to move this entry to CF 2014-12 if you are planning to continue working on it so as it would get additional review. (Note that this patch status was "Waiting on Author" when writing this text). Regards, -- Michael
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
