From 2913e8189adf9d75bdefef3c62ad83dabf2fa5cb Mon Sep 17 00:00:00 2001 From: Tatsuo Ishii Date: Mon, 13 Oct 2025 13:39:04 +0900 Subject: [PATCH v2] Allow multiple WinGetFuncArgInPartition/Frame calls with IGNORE NULLS option. Previously it was assumed that there's only one call to WinGetFuncArgInPartition/Frame in a window function when IGNORE NULLS option is specified. To allow multiple calls to them, winobj->notnull_info is modified from "uint8 *" to "uint8 **" so that winobj->notnull_info could store pointers to not null info that correspond to each function argument. Also fix the set mark position logic in WinGetFuncArgInPartition to not raise a "cannot fetch row before WindowObject's mark position" error in IGNORE NULLS case. --- src/backend/executor/nodeWindowAgg.c | 87 +++++++++++++++++++--------- 1 file changed, 61 insertions(+), 26 deletions(-) diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c index e6a53f95391..d25ab51ca82 100644 --- a/src/backend/executor/nodeWindowAgg.c +++ b/src/backend/executor/nodeWindowAgg.c @@ -69,7 +69,7 @@ typedef struct WindowObjectData int readptr; /* tuplestore read pointer for this fn */ int64 markpos; /* row that markptr is positioned on */ int64 seekpos; /* row that readptr is positioned on */ - uint8 *notnull_info; /* not null info */ + uint8 **notnull_info; /* not null info for each func args */ int num_notnull_info; /* track size of the notnull_info array */ /* @@ -214,10 +214,10 @@ static Datum ignorenulls_getfuncarginframe(WindowObject winobj, int argno, static Datum gettuple_eval_partition(WindowObject winobj, int argno, int64 abs_pos, bool *isnull, bool *isout); -static void init_notnull_info(WindowObject winobj); -static void grow_notnull_info(WindowObject winobj, int64 pos); -static uint8 get_notnull_info(WindowObject winobj, int64 pos); -static void put_notnull_info(WindowObject winobj, int64 pos, bool isnull); +static void init_notnull_info(WindowObject winobj, WindowStatePerFunc perfuncstate); +static void grow_notnull_info(WindowObject winobj, int64 pos, int argno); +static uint8 get_notnull_info(WindowObject winobj, int64 pos, int argno); +static void put_notnull_info(WindowObject winobj, int64 pos, int argno, bool isnull); /* * Not null info bit array consists of 2-bit items @@ -1304,9 +1304,14 @@ begin_partition(WindowAggState *winstate) /* reset null map */ if (winobj->ignore_nulls == IGNORE_NULLS) - memset(winobj->notnull_info, 0, - NN_POS_TO_BYTES( - perfuncstate->winobj->num_notnull_info)); + { + int numargs = perfuncstate->numArguments; + + for (int j = 0; j < numargs; j++) + memset(winobj->notnull_info[j], 0, + NN_POS_TO_BYTES( + perfuncstate->winobj->num_notnull_info)); + } } } @@ -2734,7 +2739,7 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags) winobj->localmem = NULL; perfuncstate->winobj = winobj; winobj->ignore_nulls = wfunc->ignore_nulls; - init_notnull_info(winobj); + init_notnull_info(winobj, perfuncstate); /* It's a real window function, so set up to call it. */ fmgr_info_cxt(wfunc->winfnoid, &perfuncstate->flinfo, @@ -3386,7 +3391,7 @@ ignorenulls_getfuncarginframe(WindowObject winobj, int argno, if (isout) *isout = false; - v = get_notnull_info(winobj, abs_pos); + v = get_notnull_info(winobj, abs_pos, argno); if (v == NN_NULL) /* this row is known to be NULL */ goto advance; @@ -3404,7 +3409,7 @@ ignorenulls_getfuncarginframe(WindowObject winobj, int argno, notnull_offset++; /* record the row status */ - put_notnull_info(winobj, abs_pos, *isnull); + put_notnull_info(winobj, abs_pos, argno, *isnull); } else /* this row is known to be NOT NULL */ { @@ -3444,16 +3449,20 @@ out_of_frame: * Initialize non null map. */ static void -init_notnull_info(WindowObject winobj) +init_notnull_info(WindowObject winobj, WindowStatePerFunc perfuncstate) { /* initial number of notnull info members */ #define INIT_NOT_NULL_INFO_NUM 128 + int numargs = perfuncstate->numArguments; if (winobj->ignore_nulls == PARSER_IGNORE_NULLS) { Size size = NN_POS_TO_BYTES(INIT_NOT_NULL_INFO_NUM); - winobj->notnull_info = palloc0(size); + winobj->notnull_info = palloc(sizeof(uint8 *) * numargs); + for (int i = 0; i < numargs; i++) + /* allocate notnull_info for each argument */ + winobj->notnull_info[i] = palloc0(size); winobj->num_notnull_info = INIT_NOT_NULL_INFO_NUM; } } @@ -3462,9 +3471,10 @@ init_notnull_info(WindowObject winobj) * grow_notnull_info * expand notnull_info if necessary. * pos: not null info position + * argno: argument number */ static void -grow_notnull_info(WindowObject winobj, int64 pos) +grow_notnull_info(WindowObject winobj, int64 pos, int argno) { if (pos >= winobj->num_notnull_info) { @@ -3473,8 +3483,8 @@ grow_notnull_info(WindowObject winobj, int64 pos) Size oldsize = NN_POS_TO_BYTES(winobj->num_notnull_info); Size newsize = oldsize * 2; - winobj->notnull_info = - repalloc0(winobj->notnull_info, oldsize, newsize); + winobj->notnull_info[argno] = + repalloc0(winobj->notnull_info[argno], oldsize, newsize); winobj->num_notnull_info = NN_BYTES_TO_POS(newsize); if (winobj->num_notnull_info > pos) break; @@ -3486,16 +3496,19 @@ grow_notnull_info(WindowObject winobj, int64 pos) * get_notnull_info * retrieve a map * pos: map position + * argno: argument number */ static uint8 -get_notnull_info(WindowObject winobj, int64 pos) +get_notnull_info(WindowObject winobj, int64 pos, int argno) { + uint8 *mbp; uint8 mb; int64 bpos; - grow_notnull_info(winobj, pos); + grow_notnull_info(winobj, pos, argno); bpos = NN_POS_TO_BYTES(pos); - mb = winobj->notnull_info[bpos]; + mbp = winobj->notnull_info[argno]; + mb = mbp[bpos]; return (mb >> (NN_SHIFT(pos))) & NN_MASK; } @@ -3503,22 +3516,26 @@ get_notnull_info(WindowObject winobj, int64 pos) * put_notnull_info * update map * pos: map position + * argno: argument number + * isnull: indicate NULL or NOT */ static void -put_notnull_info(WindowObject winobj, int64 pos, bool isnull) +put_notnull_info(WindowObject winobj, int64 pos, int argno, bool isnull) { + uint8 *mbp; uint8 mb; int64 bpos; uint8 val = isnull ? NN_NULL : NN_NOTNULL; int shift; - grow_notnull_info(winobj, pos); + grow_notnull_info(winobj, pos, argno); bpos = NN_POS_TO_BYTES(pos); - mb = winobj->notnull_info[bpos]; + mbp = winobj->notnull_info[argno]; + mb = mbp[bpos]; shift = NN_SHIFT(pos); mb &= ~(NN_MASK << shift); /* clear map */ mb |= (val << shift); /* update map */ - winobj->notnull_info[bpos] = mb; + mbp[bpos] = mb; } /*********************************************************************** @@ -3717,6 +3734,7 @@ WinGetFuncArgInPartition(WindowObject winobj, int argno, { WindowAggState *winstate; int64 abs_pos; + int64 mark_pos; Datum datum; bool null_treatment; int notnull_offset; @@ -3772,6 +3790,23 @@ WinGetFuncArgInPartition(WindowObject winobj, int argno, forward = relpos > 0 ? 1 : -1; myisout = false; datum = 0; + /* + * IGNORE NULLS + WINDOW_SEEK_CURRENT + relpos > 0 case, we would fetch + * beyond the current row + relpos to find out the target row. If we mark + * at abs_pos, next call to WinGetFuncArgInPartition or + * WinGetFuncArgInFrame (in case when a window function have multiple + * args) could fail with "cannot fetch row before WindowObject's mark + * position". So keep the mark position at currentpos. + */ + if (seektype == WINDOW_SEEK_CURRENT && relpos > 0) + mark_pos = winstate->currentpos; + else + /* + * For other cases we have no idea what position of row callers would + * fetch next time. Also for relpos < 0 case (we go backward), we + * cannot set mark either. For those cases we always set mark at 0. + */ + mark_pos = 0; /* * Get the next nonnull value in the partition, moving forward or backward @@ -3787,7 +3822,7 @@ WinGetFuncArgInPartition(WindowObject winobj, int argno, break; /* check NOT NULL cached info */ - nn_info = get_notnull_info(winobj, abs_pos); + nn_info = get_notnull_info(winobj, abs_pos, argno); if (nn_info == NN_NOTNULL) /* this row is known to be NOT NULL */ notnull_offset++; else if (nn_info == NN_NULL) /* this row is known to be NULL */ @@ -3802,7 +3837,7 @@ WinGetFuncArgInPartition(WindowObject winobj, int argno, if (!*isnull) notnull_offset++; /* record the row status */ - put_notnull_info(winobj, abs_pos, *isnull); + put_notnull_info(winobj, abs_pos, argno, *isnull); } } while (notnull_offset < notnull_relpos); @@ -3810,7 +3845,7 @@ WinGetFuncArgInPartition(WindowObject winobj, int argno, datum = gettuple_eval_partition(winobj, argno, abs_pos, isnull, &myisout); if (!myisout && set_mark) - WinSetMarkPosition(winobj, abs_pos); + WinSetMarkPosition(winobj, mark_pos); if (isout) *isout = myisout; -- 2.43.0