Skip to content

Commit 19b861f

Browse files
david-rowleydavid-rowley
committed
Speedup WindowAgg code by moving uncommon code out-of-line
The code to calculate the frame offsets is only performed once per scan. Moving this code out of line gives a small (around 4-5%) speedup when testing with some CPUs. Other tested CPUs are indifferent to the change. Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Reviewed-by: Tatsuo Ishii <ishii@postgresql.org> Discussion: https://postgr.es/m/CAApHDvqPgFtwme2Zyf75BpMLwYr2mnUstDyPiP%3DEpudYuQTPPQ%40mail.gmail.com
1 parent 06421b0 commit 19b861f

File tree

1 file changed

+78
-62
lines changed

1 file changed

+78
-62
lines changed

‎src/backend/executor/nodeWindowAgg.c

Lines changed: 78 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -2032,6 +2032,82 @@ update_grouptailpos(WindowAggState *winstate)
20322032
MemoryContextSwitchTo(oldcontext);
20332033
}
20342034

2035+
/*
2036+
* calculate_frame_offsets
2037+
* Determine the startOffsetValue and endOffsetValue values for the
2038+
* WindowAgg's frame options.
2039+
*/
2040+
static pg_noinline void
2041+
calculate_frame_offsets(PlanState *pstate)
2042+
{
2043+
WindowAggState *winstate = castNode(WindowAggState, pstate);
2044+
ExprContext *econtext;
2045+
int frameOptions = winstate->frameOptions;
2046+
Datum value;
2047+
bool isnull;
2048+
int16 len;
2049+
bool byval;
2050+
2051+
/* Ensure we've not been called before for this scan */
2052+
Assert(winstate->all_first);
2053+
2054+
econtext = winstate->ss.ps.ps_ExprContext;
2055+
2056+
if (frameOptions & FRAMEOPTION_START_OFFSET)
2057+
{
2058+
Assert(winstate->startOffset != NULL);
2059+
value = ExecEvalExprSwitchContext(winstate->startOffset,
2060+
econtext,
2061+
&isnull);
2062+
if (isnull)
2063+
ereport(ERROR,
2064+
(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
2065+
errmsg("frame starting offset must not be null")));
2066+
/* copy value into query-lifespan context */
2067+
get_typlenbyval(exprType((Node *) winstate->startOffset->expr),
2068+
&len,
2069+
&byval);
2070+
winstate->startOffsetValue = datumCopy(value, byval, len);
2071+
if (frameOptions & (FRAMEOPTION_ROWS | FRAMEOPTION_GROUPS))
2072+
{
2073+
/* value is known to be int8 */
2074+
int64 offset = DatumGetInt64(value);
2075+
2076+
if (offset < 0)
2077+
ereport(ERROR,
2078+
(errcode(ERRCODE_INVALID_PRECEDING_OR_FOLLOWING_SIZE),
2079+
errmsg("frame starting offset must not be negative")));
2080+
}
2081+
}
2082+
2083+
if (frameOptions & FRAMEOPTION_END_OFFSET)
2084+
{
2085+
Assert(winstate->endOffset != NULL);
2086+
value = ExecEvalExprSwitchContext(winstate->endOffset,
2087+
econtext,
2088+
&isnull);
2089+
if (isnull)
2090+
ereport(ERROR,
2091+
(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
2092+
errmsg("frame ending offset must not be null")));
2093+
/* copy value into query-lifespan context */
2094+
get_typlenbyval(exprType((Node *) winstate->endOffset->expr),
2095+
&len,
2096+
&byval);
2097+
winstate->endOffsetValue = datumCopy(value, byval, len);
2098+
if (frameOptions & (FRAMEOPTION_ROWS | FRAMEOPTION_GROUPS))
2099+
{
2100+
/* value is known to be int8 */
2101+
int64 offset = DatumGetInt64(value);
2102+
2103+
if (offset < 0)
2104+
ereport(ERROR,
2105+
(errcode(ERRCODE_INVALID_PRECEDING_OR_FOLLOWING_SIZE),
2106+
errmsg("frame ending offset must not be negative")));
2107+
}
2108+
}
2109+
winstate->all_first = false;
2110+
}
20352111

20362112
/* -----------------
20372113
* ExecWindowAgg
@@ -2061,68 +2137,8 @@ ExecWindowAgg(PlanState *pstate)
20612137
* rescan). These are assumed to hold constant throughout the scan; if
20622138
* user gives us a volatile expression, we'll only use its initial value.
20632139
*/
2064-
if (winstate->all_first)
2065-
{
2066-
int frameOptions = winstate->frameOptions;
2067-
Datum value;
2068-
bool isnull;
2069-
int16 len;
2070-
bool byval;
2071-
2072-
econtext = winstate->ss.ps.ps_ExprContext;
2073-
2074-
if (frameOptions & FRAMEOPTION_START_OFFSET)
2075-
{
2076-
Assert(winstate->startOffset != NULL);
2077-
value = ExecEvalExprSwitchContext(winstate->startOffset,
2078-
econtext,
2079-
&isnull);
2080-
if (isnull)
2081-
ereport(ERROR,
2082-
(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
2083-
errmsg("frame starting offset must not be null")));
2084-
/* copy value into query-lifespan context */
2085-
get_typlenbyval(exprType((Node *) winstate->startOffset->expr),
2086-
&len, &byval);
2087-
winstate->startOffsetValue = datumCopy(value, byval, len);
2088-
if (frameOptions & (FRAMEOPTION_ROWS | FRAMEOPTION_GROUPS))
2089-
{
2090-
/* value is known to be int8 */
2091-
int64 offset = DatumGetInt64(value);
2092-
2093-
if (offset < 0)
2094-
ereport(ERROR,
2095-
(errcode(ERRCODE_INVALID_PRECEDING_OR_FOLLOWING_SIZE),
2096-
errmsg("frame starting offset must not be negative")));
2097-
}
2098-
}
2099-
if (frameOptions & FRAMEOPTION_END_OFFSET)
2100-
{
2101-
Assert(winstate->endOffset != NULL);
2102-
value = ExecEvalExprSwitchContext(winstate->endOffset,
2103-
econtext,
2104-
&isnull);
2105-
if (isnull)
2106-
ereport(ERROR,
2107-
(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
2108-
errmsg("frame ending offset must not be null")));
2109-
/* copy value into query-lifespan context */
2110-
get_typlenbyval(exprType((Node *) winstate->endOffset->expr),
2111-
&len, &byval);
2112-
winstate->endOffsetValue = datumCopy(value, byval, len);
2113-
if (frameOptions & (FRAMEOPTION_ROWS | FRAMEOPTION_GROUPS))
2114-
{
2115-
/* value is known to be int8 */
2116-
int64 offset = DatumGetInt64(value);
2117-
2118-
if (offset < 0)
2119-
ereport(ERROR,
2120-
(errcode(ERRCODE_INVALID_PRECEDING_OR_FOLLOWING_SIZE),
2121-
errmsg("frame ending offset must not be negative")));
2122-
}
2123-
}
2124-
winstate->all_first = false;
2125-
}
2140+
if (unlikely(winstate->all_first))
2141+
calculate_frame_offsets(pstate);
21262142

21272143
/* We need to loop as the runCondition or qual may filter out tuples */
21282144
for (;;)

0 commit comments

Comments
 (0)