Re: Remove WindowClause PARTITION BY items belonging to redundant pathkeys

On Fri, Jun 9, 2023 at 8:13 AM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:

> After looking again at nodeWindowAgg.c, I think it might be possible
> to do a bit more work and apply this to ORDER BY items too. Without
> an ORDER BY clause, all rows in the partition are peers of each other,
> and if the ORDER BY column is redundant due to belonging to a
> redundant pathkey, then those rows must also be peers too since the
> redundant pathkey must mean all rows have an equal value in the
> redundant column.
>
> However, there is a case where we must be much more careful. The
> comment you highlighted in create_windowagg_plan() does mention this.
> It reads "we must *not* remove the ordering column for RANGE OFFSET
> cases".

I see. I tried to run the query below

select a, b, sum(a) over (order by b range between 10 preceding and current
row) from t where b = 2;
server closed the connection unexpectedly

and if we've removed redundant items in wc->orderClause the query would
trigger the Assert in update_frameheadpos().

/* We must have an ordering column */
Assert(node->ordNumCols == 1);

>
> It might be possible to make adjustments in nodeWindowAgg.c to have
> the equality checks come out as true when there is no ORDER BY.
> update_frameheadpos() is one location that would need to be adjusted.
> It would need further study to ensure we don't accidentally break
> anything. I've not done that study, so won't be adjusting the patch
> for now.

I'm also not sure if doing that is safe in all cases. Hmm, do you think
we can instead check wc->frameOptions to see if it is the RANGE OFFSET
case in make_pathkeys_for_window(), and decide to not remove or remove
redundant ORDER BY items according to whether it is or not RANGE OFFSET?

Thanks
Richard