On Sun, Oct 02, 2022 at 02:11:12PM -0400, Tom Lane wrote:
> "Jonathan S. Katz" <jkatz@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
