| From: | Tatsuo Ishii <ishii(at)postgresql(dot)org> |
|---|---|
| To: | assam258(at)gmail(dot)com, jian(dot)universality(at)gmail(dot)com |
| Cc: | pgsql-hackers(at)postgresql(dot)org, zsolt(dot)parragi(at)percona(dot)com, sjjang112233(at)gmail(dot)com, vik(at)postgresfriends(dot)org, er(at)xs4all(dot)nl, jacob(dot)champion(at)enterprisedb(dot)com, david(dot)g(dot)johnston(at)gmail(dot)com, peter(at)eisentraut(dot)org, li(dot)evan(dot)chao(at)gmail(dot)com |
| Subject: | Re: Row pattern recognition |
| Date: | 2026-07-01 12:18:05 |
| Message-ID: | 20260701.211805.2273383346456030984.ishii@postgresql.org |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> Will continue reviewes tomorrow.
Sorry for delay. Here is the review for
v50-0005-match-once-per-row.patch.
--- a/src/backend/executor/nodeWindowAgg.c
+++ b/src/backend/executor/nodeWindowAgg.c
@@ -239,6 +239,10 @@ static int64 row_is_in_reduced_frame(WindowObject winobj, int64 pos);
static void clear_reduced_frame(WindowAggState *winstate);
static int get_reduced_frame_status(WindowAggState *winstate, int64 pos);
+static void advance_nav_mark(WindowAggState *winstate, int64 currentPos);
+static void advance_reduced_frame_nfa(WindowObject winobj,
+ RPRNFAContext *targetCtx, int64 pos,
+ bool hasLimitedFrame, int64 frameOffset);
static void update_reduced_frame(WindowObject winobj, int64 pos);
/* Forward declarations - NFA row evaluation */
@@ -2521,6 +2525,16 @@ ExecWindowAgg(PlanState *pstate)
{
if (winstate->rpSkipTo == ST_NEXT_ROW)
clear_reduced_frame(winstate);
+
+ /*
+ * Drive the row pattern match every row, so it tracks the row
+ * scan rather than frame access: a window function that skips
+ * the frame (e.g. nth_value() with a NULL offset) must not
+ * leave the match state behind currentpos.
+ */
+ Assert(winstate->nav_winobj != NULL);
+ (void) row_is_in_reduced_frame(winstate->nav_winobj,
+ winstate->currentpos);
Is there any reason that we call row_is_in_reduced_frame() here,
instead of something like:
update_frameheadpos(winstate);
update_reduced_frame(winobj, pos);
row_is_in_reduced_frame() returns row status. Ignoring it using (void)
looks a little bit confusing to me.
Regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Jacob Champion | 2026-07-01 12:28:44 | Re: Coverage (lcov) failing with inconsistent error in versions 2.x |
| Previous Message | Ajit Awekar | 2026-07-01 12:15:20 | Re: Continuous re-validation of session credentials |