Re: enable_incremental_sort changes query behavior

On Tue, 6 Oct 2020 at 06:46, James Coleman <jtc331(at)gmail(dot)com> wrote:
>
> > All right, here's a modified patch. I did a bit of surgery: the
> > concept is the same, but I decided to explicitly not the parallels to
> > (and follow as possible) prepare_sort_from_pathkeys(). That means we
> > only have to do the expression equality check if the expr is volatile,
> > and the relids bms check doesn't have to bail if it's empty.
> >
> > This properly allows us to push the sort all the way down to the base
> > rel when the only things in the base rel are referenced (including
> > stable expressions) but pushes the sort up to the upper rel when a
> > volatile expression is used (technically it would be safe for it to go
> > lower, but in the current planner that's the only time it appears in
> > the target list, so allowing that would be a larger functional
> > change).
> >
> > I left the questions patch here, though obviously it doesn't need to
> > be committed. If someone has answers to the questions, I'd be
> > interested though, but I don't think it affects the correctness of
> > this patch.
>
> And with a typo fixed.
>

Hi James,

Thanks for working on this, I haven't tried the latest patch but the
previous single patch worked fine in the cases you and Tomas found and
with my original example.

I haven't been able to reproduce the same problem with other queries
(I tried RLS and views with barriers, i also wanted to test with LEAK
functions but didn't have the time yet).

Can you please create an entry in the commitfest for this one so we
don't lose track of it?

--
Jaime Casanova www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services