On Fri, Oct 2, 2020 at 6:28 PM Tomas Vondra <tomas.von...@2ndquadrant.com> wrote: > > On Fri, Oct 02, 2020 at 05:45:52PM -0400, James Coleman wrote: > >On Fri, Oct 2, 2020 at 4:56 PM Tomas Vondra > ><tomas.von...@2ndquadrant.com> wrote: > >> > >> ... > >> > >> More importanly, it does not actually fix the issue - it does fix that > >> particular query, but just replacing the DISTINCT with either ORDER BY > >> or GROUP BY make it fail again :-( > >> > >> Attached is a simple script I used, demonstrating these issues for the > >> three cases that expect to have ressortgroupref != 0 (per the comment > >> before TargetEntry in plannodes.h). > > > >So does checking for volatile expressions (if you happened to test > >that) solve all the cases? If you haven't tested that yet, I can try > >to do that this evening. > > > > Yes, it does fix all the three queries in the SQL script. > > The question however is whether this is the root issue, and whether it's > the right way to fix it. For example - volatility is not the only reason > that may block qual pushdown. If you look at qual_is_pushdown_safe, it > also blocks pushdown of leaky functions in security_barrier views. So I > wonder if that could cause failures too, somehow. But I haven't managed > to create such example.
I was about to say that the issue here is slightly different from qual etc. pushdown, since we're not concerned about quals here, and so I wonder where we determine what target list entries to put in a given scan path, but then I realized that implies (maybe!) a simpler solution. Instead of duplicating checks on target list entries would be safe, why not check directly in get_useful_pathkeys_for_relation() whether or not the pathkey has a target list entry? I haven't been able to try that out yet, and so maybe I'm missing something, but I need to step out for a bit, so I'll have to look at it later. James
