postgres.git / summary / log / commit / refs

commit    829bacaeb323e4811eb804b5e4e29f643a0c619d
Author:   Tatsuo Ishii <ishii@postgresql.org>
Date:     Sun Jul 19 01:47:29 2026 +0000

    Remove redundant null-treatment check in window function dedup.
    
    Commit 25a30bbd423 (IGNORE NULLS / RESPECT NULLS for window functions)
    made ExecInitWindowAgg() treat two otherwise-equal window functions as
    duplicates only when their ignore_nulls settings also matched:
    
        if (i <= wfuncno && wfunc->ignore_nulls == perfunc[i].ignore_nulls)
    
    That extra term reads WindowStatePerFuncData.ignore_nulls, but the field
    was never populated when a per-function entry was filled in, so it stayed
    zero from palloc0_array().  Consequently a duplicate call carrying
    IGNORE NULLS or an explicit RESPECT NULLS never matched an identical
    earlier entry and was needlessly given its own per-function slot and
    evaluated twice.  (Results stayed correct; this was a missed sharing, not
    a wrong answer.)
    
    The extra term is in fact redundant.  WindowFunc.ignore_nulls is a plain
    scalar field with no pg_node_attr, so _equalWindowFunc() already compares
    it; the preceding equal() call therefore never matches two WindowFuncs
    that differ only in null treatment.  If equal() matches, ignore_nulls
    necessarily matched too, so the term can never change the outcome, and
    WindowStatePerFuncData.ignore_nulls existed only to feed it.
    
    Rather than populate the shadow field, drop the redundant term and the
    field (and adjust the now-stale comment) and let equal() do the work.
    That fixes the same bug while removing the hand-maintained duplicate
    state that caused it, so it cannot silently drift again.
    
    Author: Chao Li <li.evan.chao@gmail.com>
    Co-authored-by: Ewan Young <kdbase.hack@gmail.com>
    Reviewed-by: Tatsuo Ishii <ishii@postgresql.org>
    Discussion: https://postgr.es/m/5D2C9081-5DFE-4E27-AB14-7358238EA1BC%40gmail.com
    Backpatch-through: 19


 src/backend/executor/nodeWindowAgg.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c
index f1c524d00df..7d6ec2dfc4b 100644
--- a/src/backend/executor/nodeWindowAgg.c
+++ b/src/backend/executor/nodeWindowAgg.c
@@ -110,7 +110,6 @@ typedef struct WindowStatePerFuncData
 
 	bool		plain_agg;		/* is it just a plain aggregate function? */
 	int			aggno;			/* if so, index of its WindowStatePerAggData */
-	uint8		ignore_nulls;	/* ignore nulls */
 
 	WindowObject winobj;		/* object used in window function API */
 } WindowStatePerFuncData;
@@ -2737,17 +2736,14 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags)
 			elog(ERROR, "WindowFunc with winref %u assigned to WindowAgg with winref %u",
 				 wfunc->winref, node->winref);
 
-		/*
-		 * Look for a previous duplicate window function, which needs the same
-		 * ignore_nulls value
-		 */
+		/* Look for a previous duplicate window function */
 		for (i = 0; i <= wfuncno; i++)
 		{
 			if (equal(wfunc, perfunc[i].wfunc) &&
 				!contain_volatile_functions((Node *) wfunc))
 				break;
 		}
-		if (i <= wfuncno && wfunc->ignore_nulls == perfunc[i].ignore_nulls)
+		if (i <= wfuncno)
 		{
 			/* Found a match to an existing entry, so just mark it */
 			wfuncstate->wfuncno = i;


[parent: f7bc98156e11]