Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit a767cdc

Browse filesBrowse files
tglsfdctglsfdc
committed
Fix unnecessary use of moving-aggregate mode with non-moving frame.
When a plain aggregate is used as a window function, and the window frame start is specified as UNBOUNDED PRECEDING, the frame's head cannot move so we do not need to use moving-aggregate mode. The check for that was put into initialize_peragg(), failing to notice that ExecInitWindowAgg() calls that function before it's filled in winstate->frameOptions. Since makeNode() would have zeroed the field, this didn't provoke uninitialized-value complaints, nor would the erroneous decision have resulted in more than a little inefficiency. Still, it's wrong, so move the initialization of winstate->frameOptions earlier to make it work properly. While here, also fix a thinko in a comment. Both errors crept in in commit a9d9acb which introduced the moving-aggregate mode. Spotted by Vallimaharajan G. Back-patch to all supported branches. Discussion: https://postgr.es/m/18e7f2a5167.fe36253866818.977923893562469143@zohocorp.com
1 parent 44a4cca commit a767cdc
Copy full SHA for a767cdc

File tree

Expand file treeCollapse file tree

1 file changed

+4
-4
lines changed
Filter options
Expand file treeCollapse file tree

1 file changed

+4
-4
lines changed

‎src/backend/executor/nodeWindowAgg.c

Copy file name to clipboardExpand all lines: src/backend/executor/nodeWindowAgg.c
+4-4Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2399,6 +2399,9 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags)
23992399
winstate->ss.ps.state = estate;
24002400
winstate->ss.ps.ExecProcNode = ExecWindowAgg;
24012401

2402+
/* copy frame options to state node for easy access */
2403+
winstate->frameOptions = frameOptions;
2404+
24022405
/*
24032406
* Create expression contexts. We need two, one for per-input-tuple
24042407
* processing and one for per-output-tuple processing. We cheat a little
@@ -2649,9 +2652,6 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags)
26492652
/* Set the status to running */
26502653
winstate->status = WINDOWAGG_RUN;
26512654

2652-
/* copy frame options to state node for easy access */
2653-
winstate->frameOptions = frameOptions;
2654-
26552655
/* initialize frame bound offset expressions */
26562656
winstate->startOffset = ExecInitExpr((Expr *) node->startOffset,
26572657
(PlanState *) winstate);
@@ -2785,7 +2785,7 @@ initialize_peragg(WindowAggState *winstate, WindowFunc *wfunc,
27852785

27862786
/*
27872787
* Figure out whether we want to use the moving-aggregate implementation,
2788-
* and collect the right set of fields from the pg_attribute entry.
2788+
* and collect the right set of fields from the pg_aggregate entry.
27892789
*
27902790
* It's possible that an aggregate would supply a safe moving-aggregate
27912791
* implementation and an unsafe normal one, in which case our hand is

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.