Hi, On 2020-06-23 09:23:57 -0700, Melanie Plageman wrote: > On Mon, Jun 22, 2020 at 9:02 PM Andres Freund <and...@anarazel.de> wrote: > > It's not this patch's fault, but none, really none, of this stuff should > > be in the executor. > > > > > Were you thinking it could be done in grouping_planner() and then the > bitmaps could be saved in the PlannedStmt? > Or would you have to wait until query_planner()? Or are you imagining > somewhere else entirely?
I haven't thought about it in too much detail, but I would say create_agg_plan() et al. I guess there's some argument to be made to do it in setrefs.c, because we already do convert_combining_aggrefs() there (but I don't like that much). There's no reason to do it before we actually decided on one specific path, so doing it earlier than create_plan() seems unnecessary. And having it in agg specific code seems better than putting it into global routines. There's probably an argument for having a bit more shared code between create_agg_plan(), create_group_plan() and create_groupingsets_plan(). But even just adding a new extract_*_cols() call to each of those would probably be ok. Greetings, Andres Freund
