From: | "Jonathan S(dot) Katz" <jkatz(at)postgresql(dot)org> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, "kuroda(dot)hayato(at)fujitsu(dot)com" <kuroda(dot)hayato(at)fujitsu(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, John Naylor <john(dot)naylor(at)enterprisedb(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz> |
Subject: | Re: Question: test "aggregates" failed in 32-bit machine |
Date: | 2022-10-02 16:36:52 |
Message-ID: | 1ae15468-ea71-4d75-b096-cc2cb2e8cd45@postgresql.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 10/1/22 6:57 PM, Tom Lane wrote:
> "Jonathan S. Katz" <jkatz(at)postgresql(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2022-10-02 16:49:31 | ssl tests aren't concurrency safe due to get_free_port() |
Previous Message | Andres Freund | 2022-10-02 16:04:13 | Re: proposal: possibility to read dumped table's name from file |