Re: enable_incremental_sort changes query behavior
От | Tomas Vondra |
---|---|
Тема | Re: enable_incremental_sort changes query behavior |
Дата | |
Msg-id | 20201103213942.sj3qohewdshrw4oz@development обсуждение исходный текст |
Ответ на | Re: enable_incremental_sort changes query behavior (Tomas Vondra <tomas.vondra@2ndquadrant.com>) |
Ответы |
Re: enable_incremental_sort changes query behavior
Re: enable_incremental_sort changes query behavior |
Список | pgsql-hackers |
On Tue, Nov 03, 2020 at 03:37:43AM +0100, Tomas Vondra wrote: >On Fri, Oct 30, 2020 at 07:37:33PM -0400, James Coleman wrote: >> >>... >> >>I was looking at this some more, and I'm still convinced that this is >>correct, but I don't think a comment about it being an optimization >>(though I suspect that that its major benefit), since it came from, >>and must parallel, find_ec_member_for_tle, and there is no such >>explanation there. I don't want to backfill reasoning onto that >>decision, but I do think it's important to parallel it since it's >>ultimately what is going to cause errors if we're out of sync with it. >> >>As to why find_em_expr_for_rel is different, I think the answer is >>that it's used by the FDW code, and it seems to me to make sense that >>in that case we'd only send sort conditions to the foreign server if >>the em actually contains rels. >> >>The new series attached includes the primary fix for this issue, the >>additional comments that could either go in at the same time or not, >>and my patch with semi-related questions (which isn't intended to be >>committed, but to keep track of the questions). >> > >Thanks. Attached are two patches that I plan to get committed > >0001 is what you sent, with slightly reworded commit message. This is >the actual fix. > >I'm still thinking about Robert's comment that perhaps we should be >considering only expressions already in the relation's target, which >would imply checking for volatile expressions is not enough. But I've >been unable to convince myself it's correct or incorrect. In any case >0001 is a clear improvement - in the worst case we'll have to make it >stricter in the future. > > >0002 partially reverts ba3e76cc571 by moving find_em_expr_for_rel back >to postgres_fdw.c. We've moved it to equivclass.c so that it could be >called from both the FDW and allpaths.c, but now that the functions >diverged it's only called from the FDW again. So maybe we should move >it back, but I guess that's not a good thing in a minor release. > I've pushed the 0001 part, i.e. the main fix. Not sure about the other parts (comments and moving the code back to postgres_fdw) yet. regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
В списке pgsql-hackers по дате отправления: