Re: BUG #17495: Regression in 15beta1 when filtering subquery including row_number window function

On Wed, 25 May 2022 at 14:35, Richard Guo <guofenglinux@gmail.com> wrote:
> This should be introduced in by 9d9c02cc. The rel of the subquery loses
> its baserestrictinfo, which is (x_sequel_row_number_x <= 2), when
> generating SubqueryScan access paths for it.  As a result, the
> WindowFunc target entry is removed as an unused targetlist item.

Thanks for looking into that.

FWIW, a simplified test case is:

explain select x from (select x,row_number() over (order by x) rn from
generate_Series(1,100) x) gs where gs.rn < 10;

I think the correct fix is to change remove_unused_subquery_outputs()
so it also checks the WindowClause runConditions in addition to the
rel targetlist and baserestrictinfo.

I played around with the attached (rather horrible) patch. The problem
I have with the patch is that we can't simply call pull_varattnos() on
the WindowClause.runCondition. This is because the runCondition
OpExprs contain WindowFunc references rather than Vars which reference
one of those in the targetlist.  To make this work I had to trawl
through the targetlist of the subquery and look for an entry which is
equal to the runCondition's WindowFunc reference.  This is pretty
horrible as we've no way to know that we've found the right one.

The following query shows the problem:

explain verbose select x from (select x,row_number() over (order by x)
as dummy, row_number() over (order by x) rn from
generate_Series(1,100) x) gs where gs.rn < 10;

Looking at the output of the WindowAgg shows that the "dummy" column
was kept and "rn" was replaced with the NULL Const.  This does not
seem to lead to any actual problems, but it does set off some alarm
bells.

  ->  WindowAgg  (cost=6.82..8.57 rows=100 width=20)
        Output: x.x, row_number() OVER (?), NULL::bigint


Maybe a better fix is to add a new Bitmapset field to WindowClause and
have find_window_run_conditions() record the attno in that field when
it appends the new runCondition to the runCondition field.
remove_unused_subquery_outputs() can just bms_add_members that field
to attrs_used.  This just means having to add a field to WindowClause
post-beta.  Is that going to be a problem?

David