Handle WindowClause.runCondition in tree walker/mutator functions.

tglsfdc · tglsfdc · commit ee955321869e · 2024-01-10T13:36:34.000-05:00
Commit 9d9c02c, which added the notion of a "run condition" for window functions, neglected to teach nodeFuncs.c to process the new field. Remarkably, that doesn't seem to have had any ill effects before we invented Var.varnullingrels, but now it can cause visible failures in join-removal scenarios. I have no faith that there's not reachable problems in v15 too, so back-patch the code change to v15 where 9d9c02c came in. The test case seems irrelevant to v15, though. Per bug #18277 from Zuming Jiang. Diagnosis and patch by Richard Guo. Discussion: https://postgr.es/m/18277-089ead83b329a2fd@postgresql.org
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
@@ -2283,6 +2283,8 @@ expression_tree_walker_impl(Node *node,
 					return true;
 				if (WALK(wc->endOffset))
 					return true;
+				if (WALK(wc->runCondition))
+					return true;
 			}
 			break;
 		case T_CTECycleClause:
@@ -2627,6 +2629,8 @@ query_tree_walker_impl(Query *query,
 				return true;
 			if (WALK(wc->endOffset))
 				return true;
+			if (WALK(wc->runCondition))
+				return true;
 		}
 	}
 
@@ -3312,6 +3316,7 @@ expression_tree_mutator_impl(Node *node,
 				MUTATE(newnode->orderClause, wc->orderClause, List *);
 				MUTATE(newnode->startOffset, wc->startOffset, Node *);
 				MUTATE(newnode->endOffset, wc->endOffset, Node *);
+				MUTATE(newnode->runCondition, wc->runCondition, List *);
 				return (Node *) newnode;
 			}
 			break;
@@ -3641,6 +3646,7 @@ query_tree_mutator_impl(Query *query,
 			FLATCOPY(newnode, wc, WindowClause);
 			MUTATE(newnode->startOffset, wc->startOffset, Node *);
 			MUTATE(newnode->endOffset, wc->endOffset, Node *);
+			MUTATE(newnode->runCondition, wc->runCondition, List *);
 
 			resultlist = lappend(resultlist, (Node *) newnode);
 		}
diff --git a/src/test/regress/expected/window.out b/src/test/regress/expected/window.out
@@ -3804,6 +3804,32 @@ SELECT * FROM
  sales     |     3 |   4800 | 08-01-2007  |  3 |  1 |  3 |  3 |  1
 (2 rows)
 
+-- Ensure we remove references to reduced outer joins as nulling rels in run
+-- conditions
+EXPLAIN (COSTS OFF)
+SELECT 1 FROM
+  (SELECT ntile(e2.salary) OVER (PARTITION BY e1.depname) AS c
+   FROM empsalary e1 LEFT JOIN empsalary e2 ON TRUE
+   WHERE e1.empno = e2.empno) s
+WHERE s.c = 1;
+                       QUERY PLAN                        
+---------------------------------------------------------
+ Subquery Scan on s
+   Filter: (s.c = 1)
+   ->  WindowAgg
+         Run Condition: (ntile(e2.salary) OVER (?) <= 1)
+         ->  Sort
+               Sort Key: e1.depname
+               ->  Merge Join
+                     Merge Cond: (e1.empno = e2.empno)
+                     ->  Sort
+                           Sort Key: e1.empno
+                           ->  Seq Scan on empsalary e1
+                     ->  Sort
+                           Sort Key: e2.empno
+                           ->  Seq Scan on empsalary e2
+(14 rows)
+
 -- Tests to ensure we don't push down the run condition when it's not valid to
 -- do so.
 -- Ensure we don't push down when the frame options show that the window
diff --git a/src/test/regress/sql/window.sql b/src/test/regress/sql/window.sql
@@ -1236,6 +1236,15 @@ SELECT * FROM
    FROM empsalary
 ) e WHERE rn <= 1 AND c1 <= 3 AND nt < 2;
 
+-- Ensure we remove references to reduced outer joins as nulling rels in run
+-- conditions
+EXPLAIN (COSTS OFF)
+SELECT 1 FROM
+  (SELECT ntile(e2.salary) OVER (PARTITION BY e1.depname) AS c
+   FROM empsalary e1 LEFT JOIN empsalary e2 ON TRUE
+   WHERE e1.empno = e2.empno) s
+WHERE s.c = 1;
+
 -- Tests to ensure we don't push down the run condition when it's not valid to
 -- do so.
 

Original file line number	Diff line number	Diff line change
`@@ -2283,6 +2283,8 @@ expression_tree_walker_impl(Node *node,`
`2283`	`2283`	`return true;`
`2284`	`2284`	`if (WALK(wc->endOffset))`
`2285`	`2285`	`return true;`
	`2286`	`+ if (WALK(wc->runCondition))`
	`2287`	`+ return true;`
`2286`	`2288`	`}`
`2287`	`2289`	`break;`
`2288`	`2290`	`case T_CTECycleClause:`
`@@ -2627,6 +2629,8 @@ query_tree_walker_impl(Query *query,`
`2627`	`2629`	`return true;`
`2628`	`2630`	`if (WALK(wc->endOffset))`
`2629`	`2631`	`return true;`
	`2632`	`+ if (WALK(wc->runCondition))`
	`2633`	`+ return true;`
`2630`	`2634`	`}`
`2631`	`2635`	`}`
`2632`	`2636`
`@@ -3312,6 +3316,7 @@ expression_tree_mutator_impl(Node *node,`
`3312`	`3316`	`MUTATE(newnode->orderClause, wc->orderClause, List *);`
`3313`	`3317`	`MUTATE(newnode->startOffset, wc->startOffset, Node *);`
`3314`	`3318`	`MUTATE(newnode->endOffset, wc->endOffset, Node *);`
	`3319`	`+ MUTATE(newnode->runCondition, wc->runCondition, List *);`
`3315`	`3320`	`return (Node *) newnode;`
`3316`	`3321`	`}`
`3317`	`3322`	`break;`
`@@ -3641,6 +3646,7 @@ query_tree_mutator_impl(Query *query,`
`3641`	`3646`	`FLATCOPY(newnode, wc, WindowClause);`
`3642`	`3647`	`MUTATE(newnode->startOffset, wc->startOffset, Node *);`
`3643`	`3648`	`MUTATE(newnode->endOffset, wc->endOffset, Node *);`
	`3649`	`+ MUTATE(newnode->runCondition, wc->runCondition, List *);`
`3644`	`3650`
`3645`	`3651`	`resultlist = lappend(resultlist, (Node *) newnode);`
`3646`	`3652`	`}`