Re: Fix a reference error for window functions: In the function 'find_window_functions', the deduplication logic should be removed

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: Meng Zhang <mza117jc(at)gmail(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Fix a reference error for window functions: In the function 'find_window_functions', the deduplication logic should be removed
Date: 2026-01-25 10:14:57
Message-ID: CAApHDvpOXK6G2304mSzs9Hb110_U7SyYmoT0p9AucrEgZv5tJw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sun, 25 Jan 2026 at 20:05, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
>
> On Sun, 25 Jan 2026 at 17:09, Meng Zhang <mza117jc(at)gmail(dot)com> wrote:
> > The deduplication logic won't cause an error when the result of this function is only used in `select_active_windows`.
> > But when the result is used in `optimize_window_clauses`, it will cause the `winref` field of a certain window function to not be modified in the new window.
>
> Thanks for the report. I'll have a look.

I initially didn't think your patch was correct as it was getting rid
of the deduplication logic, but after looking a bit deeper, the
deduplication logic no longer works. I assume it did work when Tom
wrote the comment in 2016 (51c0f63e4), but in 2017 Andres changed the
expression evaluation code for projections (b8d7f053c). The expression
evaluation code goes through the targetlist and will find the
WindowFuncs again, all 3 of them, in this case, and since the
deduplication code didn't put them all in the WindowFuncLists lists,
one still has the unmodified winref because optimize_window_clauses()
only operates on the non-duplicate WindowFuncs.

The problem now is if we just delete the code as your patch does, then
the costing code will newly include costs for any duplicate
WindowFuncs, and that could result in the dreaded plan changes in the
backbranches problem. Yes, really we should be including these extra
costs as we *do* evaluate the WindowFuncs separately, so in master, I
think what you have is fine. We just probably will need to do
something to maintain the costs in the backbranches.

I came up with the attached for that. I did write the list_uniquify()
before I realised your fix is ok for master. That function might be
misplaced just in the backbranches, and it might be better to just
foreach and if (!list_member()) directly in optimize_window_clauses()
to get rid of the duplicates. That's probably safer too.

I'll park the attached patch here for a bit to see if anyone else has
any thoughts.

David

Attachment Content-Type Size
backbranch_optimize_window_clauses_fix.patch text/plain 4.1 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Corey Huinker 2026-01-25 10:20:13 Re: Extended Statistics set/restore/clear functions.
Previous Message Dean Rasheed 2026-01-25 10:03:05 Re: ABI Compliance Checker GSoC Project