On Thu, Jul 9, 2020 at 7:03 AM Stephen Frost <sfr...@snowman.net> wrote: > > The one for the planner is already there, and it looks like we need one > > for the executor as well (to tell HashAgg to ignore the memory limit > > just like v12). > > No, ignoring the limit set was, as agreed above, a bug, and I don't > think it makes sense to add some new user tunable for this.
It makes more sense than simply ignoring what our users will see as a simple regression. (Though I still lean towards fixing the problem by introducing hash_mem, which at least tries to fix the problem head on.) > If folks > want to let HashAgg use more memory then they can set work_mem higher, > just the same as if they want a Sort node to use more memory or a > HashJoin. Yes, that comes with potential knock-on effects about other > nodes (possibly) using more memory but that's pretty well understood for > all the other cases and I don't think that it makes sense to have a > special case for HashAgg when the only justification is that "well, you > see, it used to have this bug, so...". That's not the only justification. The other justification is that it's generally reasonable to prefer giving hash aggregate more memory. This would even be true in a world where all grouping estimates were somehow magically accurate. These two justifications coincide in a way that may seem a bit too convenient to truly be an accident of history. And if they do: I agree. It's no accident. It seems likely that we have been "complicit" in enabling "applications that live beyond their means", work_mem-wise. We knew that hash aggregate had this "bug" forever, and yet we were reasonably happy to have it be the common case for years. It's very fast, and didn't actually explode most of the time (even though grouping estimates are often pretty poor). Hash agg was and is the common case. Yes, we were concerned about the risk of OOM for many years, but it was considered a risk worth taking. We knew what the trade-off was. We never quite admitted it, but what does it matter? Our own tacit attitude towards hash agg + work_mem mirrors that of our users (or at least the users that will be affected by this issue, of which there will be plenty). Declaring this behavior a bug with no redeeming qualities now seems a bit rich. -- Peter Geoghegan
