On 10/1/22 6:57 PM, Tom Lane wrote:
"Jonathan S. Katz" <jk...@postgresql.org> writes:On 10/1/22 3:13 PM, Tom Lane wrote:I'm still of the opinion that we need to revert this code for now.[RMT hat, but speaking just for me] reading through Tom's analysis, this seems to be the safest path forward. I have a few questions to better understand:1. How invasive would the revert be?I've just finished constructing a draft full-reversion patch. I'm not confident in this yet; in particular, teasing it apart from 1349d2790 ("Improve performance of ORDER BY / DISTINCT aggregates") was fairly messy. I need to look through the regression test changes and make sure that none are surprising. But this is approximately the right scope if we rip it out entirely. I plan to have a look tomorrow at the idea of reverting only the cost_sort changes, and rewriting get_cheapest_group_keys_order() to just sort the keys by decreasing numgroups estimates as I suggested upthread. That might be substantially less messy, because of fewer interactions with 1349d2790.
Maybe this leads to a follow-up question of do we continue to improve what is in HEAD while reverting the code in v15 (particularly if it's easier to do it that way)?
I know we're generally not in favor of that approach, but wanted to ask.
2. Are the other user-visible items that would be impacted?See above. (But note that 1349d2790 is HEAD-only, not in v15.)
With the RMT hat, I'm hyperfocused on PG15 stability. We have plenty of time time to stabilize head for v16 :)
3. Is there an option of disabling the feature by default viable?Not one that usefully addresses my concerns. The patch did add an enable_group_by_reordering GUC which we could change to default-off, but it does nothing about the cost_sort behavioral changes. I would be a little inclined to rip out that GUC in either case, because I doubt that we need it with the more restricted change.
Understood.I'll wait for your analysis of reverting only the cost_sort changes etc. mentioned above.
Thanks, Jonathan
OpenPGP_signature
Description: OpenPGP digital signature