On Fri, Jun 9, 2023 at 8:13 AM David Rowley <dgrowle...@gmail.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
