Re: Question: test "aggregates" failed in 32-bit machine

[Jonathan S. Katz]

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.

I know we're generally not in favor of that approach, but wanted to ask.

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)?

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.



Thanks,

Jonathan

I'll wait for your analysis of reverting only the cost_sort changes etc. mentioned above.

OpenPGP_signature
Description: OpenPGP digital signature