On Tue, May 26, 2020 at 5:40 PM Jeff Davis <pg...@j-davis.com> wrote:
> On Tue, 2020-05-26 at 21:15 +0200, Tomas Vondra wrote: > > > > As for the tlist fix, I think that's mostly ready too - the one thing > > we > > should do is probably only doing it for AGG_HASHED. For AGG_SORTED > > it's > > not really necessary. > > Melanie previously posted a patch to avoid spilling unneeded columns, > but it introduced more code: > > > > https://www.postgresql.org/message-id/caakru_aefesv+ukqwqu+ioenoil2lju9diuh9br8mbyxuz0...@mail.gmail.com > > and it seems that Heikki also looked at it. Perhaps we should get an > acknowledgement from one of them that your one-line change is the right > approach? > > I spent some time looking at it today, and, it turns out I was wrong. I thought that there was a case I had found where CP_SMALL_TLIST did not eliminate as many columns as could be eliminated for the purposes of spilling, but, that turned out not to be the case. I changed CP_LABEL_TLIST to CP_SMALL_TLIST in create_groupingsets_plan(), create_agg_plan(), etc and tried a bunch of different queries and this 2-3 line change worked for all the cases I tried. Is that where you made the change? And then are you proposing to set it based on the aggstrategy to either CP_LABEL_TLIST or CP_SMALL_TLIST here? -- Melanie Plageman
