postgres.git / summary / log / commit / refs

commit    4297a35196de281e551d500fc71c7033f253cb98
Author:   David Rowley <drowley@postgresql.org>
Date:     Mon Jan 26 10:47:37 2026 +0000
First released in: 16.12, 17.8, 18.2

    Fix possible issue of a WindowFunc being in the wrong WindowClause
    
    ed1a88dda made it so WindowClauses can be merged when all window
    functions belonging to the WindowClause can equally well use some
    other WindowClause without any behavioral changes.  When that
    optimization applies, the WindowFunc's "winref" gets adjusted to
    reference the new WindowClause.
    
    That commit does not work well with the deduplication logic in
    find_window_functions(), which only added the WindowFunc to the list
    when there wasn't already an identical WindowFunc in the list.  That
    deduplication logic meant that the duplicate WindowFunc wouldn't get the
    "winref" changed when optimize_window_clauses() was able to swap the
    WindowFunc to another WindowClause.  This could lead to the following
    error in the unlikely event that the deduplication code did something and
    the duplicate WindowFunc happened to be moved into another WindowClause.
    
    ERROR:  WindowFunc with winref 2 assigned to WindowAgg with winref 1
    
    As it turns out, the deduplication logic in find_window_functions() is
    pretty bogus.  It might have done something when added, as that code
    predates b8d7f053c, which changed how projections work.  As it turns
    out, at least now we *will* evaluate the duplicate WindowFuncs.  All
    that the deduplication code seems to do today is assist in
    underestimating the WindowAggPath costs due to not counting the
    evaluation costs of duplicate WindowFuncs.
    
    Ideally the fix would be to remove the deduplication code, but that
    could result in changes to the plan costs, as duplicate WindowFuncs
    would then be costed.  Instead, let's play it safe and shift the
    deduplication code so it runs after the other processing in
    optimize_window_clauses().
    
    Backpatch only as far as v16 as there doesn't seem to be any other harm
    done by the WindowFunc deduplication code before then.  This issue was
    fixed in master by 7027dd499.
    
    Reported-by: Meng Zhang <mza117jc@gmail.com>
    Author: Meng Zhang <mza117jc@gmail.com>
    Author: David Rowley <dgrowleyml@gmail.com>
    Discussion: https://postgr.es/m/CAErYLFAuxmW0UVdgrz7iiuNrxGQnFK_OP9hBD5CUzRgjrVrz=Q@mail.gmail.com
    Backpatch-through: 16


 src/backend/optimizer/plan/planner.c | 35 +++++++++++++++++++++++++++++++++++
 src/backend/optimizer/util/clauses.c | 11 ++++-------
 2 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index d7801a13091..c0f752672f0 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -5796,6 +5796,41 @@ optimize_window_clauses(PlannerInfo *root, WindowFuncLists *wflists)
 			}
 		}
 	}
+
+	/*
+	 * XXX remove any duplicate WindowFuncs from each WindowClause.  This has
+	 * been done only in the back branches.  Previously, the deduplication was
+	 * done in find_window_functions(), but that caused issues with the code
+	 * above when moving a WindowFunc to another WindowClause as any duplicate
+	 * WindowFuncs won't receive the adjusted winref when merging
+	 * WindowClauses.  The deduplication below has been done only so that we
+	 * maintain the same cost calculations.  As it turns out, the previous
+	 * deduplication code thought it was saving effort during execution by
+	 * getting rid of duplicates, but that was not true as the expression
+	 * evaluation code will evaluate each WindowFunc mentioned in the
+	 * targetlist.
+	 */
+	foreach(lc, windowClause)
+	{
+		WindowClause *wc = lfirst_node(WindowClause, lc);
+		ListCell   *lc2;
+		List	   *list = wflists->windowFuncs[wc->winref];
+		List	   *newlist = NIL;
+
+		if (list == NIL)
+			continue;
+
+		foreach(lc2, list)
+		{
+			if (!list_member(newlist, lfirst(lc2)))
+				newlist = lappend(newlist, lfirst(lc2));
+			else
+				wflists->numWindowFuncs--;
+		}
+		list_free(list);
+
+		wflists->windowFuncs[wc->winref] = newlist;
+	}
 }
 
 /*
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 3302efc5b4b..7bb9dbbcd89 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -250,13 +250,10 @@ find_window_functions_walker(Node *node, WindowFuncLists *lists)
 		if (wfunc->winref > lists->maxWinRef)
 			elog(ERROR, "WindowFunc contains out-of-range winref %u",
 				 wfunc->winref);
-		/* eliminate duplicates, so that we avoid repeated computation */
-		if (!list_member(lists->windowFuncs[wfunc->winref], wfunc))
-		{
-			lists->windowFuncs[wfunc->winref] =
-				lappend(lists->windowFuncs[wfunc->winref], wfunc);
-			lists->numWindowFuncs++;
-		}
+
+		lists->windowFuncs[wfunc->winref] =
+			lappend(lists->windowFuncs[wfunc->winref], wfunc);
+		lists->numWindowFuncs++;
 
 		/*
 		 * We assume that the parser checked that there are no window


[parent: e7391bbf14db]