On Sun, Oct 02, 2022 at 02:11:12PM -0400, Tom Lane wrote: > "Jonathan S. Katz" <jk...@postgresql.org> writes: >> OK. For v15 I am heavily in favor for the least risky approach given the >> point we are at in the release cycle. The RMT hasn’t met yet to discuss, >> but from re-reading this thread again, I would recommend to revert >> (i.e. the “straight up revert”). > > OK by me.
I don't quite see why it would be to let this code live on HEAD if it is not ready to be merged as there is a risk of creating side issues with things tied to the costing still ready to be merged, so I agree that the reversion done on both branches is the way to go for now. This could always be reworked and reproposed in the future. > I'm just about to throw up my hands and go for reversion in both branches, > because I'm now discovering that the code I'd hoped to salvage in > pathkeys.c (get_useful_group_keys_orderings and related) has its very own > bugs. It's imagining that it can rearrange a PathKeys list arbitrarily > and then rearrange the GROUP BY SortGroupClause list to match, but that's > easier said than done, for a couple of different reasons. (I now > understand why db0d67db2 made a cowboy hack in get_eclass_for_sort_expr ... > but it's still a cowboy hack with difficult-to-foresee side effects.) > There are other things in there that make it painfully obvious that > this code wasn't very carefully reviewed, eg XXX comments that should > have been followed up and were not, or a reference to a nonexistent > "debug_group_by_match_order_by" flag (maybe that was a GUC at some point?). Okay. Ugh. -- Michael
signature.asc
Description: PGP signature