MIME-Version: 1.0
References: 
 <CAAAe_zDDuJofafXyhggPPPLzUXeejH-19NfcLR7jNNQxZchtog@mail.gmail.com>
 <20260601.111119.1029884790276077667.ishii@postgresql.org>
 <CAAAe_zBhzHyazC_Ot+HXBe_nToKc7AHs4r-s0nNcaBPw0L17wA@mail.gmail.com>
 <20260601.114703.1561993497414705173.ishii@postgresql.org>
 <CAAAe_zCZc8s4zWfmVVUOt0y_FU=v7YTcJJJ4UL2gBzJ2_KkUmQ@mail.gmail.com>
 <CACJufxHVADC8e77pnQxSZRk7SYHCZFk6ZCM2HfTsKyD_kUji0A@mail.gmail.com>
In-Reply-To: 
 <CACJufxHVADC8e77pnQxSZRk7SYHCZFk6ZCM2HfTsKyD_kUji0A@mail.gmail.com>
Reply-To: assam258@gmail.com
From: Henson Choi <assam258@gmail.com>
Date: Wed, 3 Jun 2026 07:14:21 +0900
Message-ID: 
 <CAAAe_zAZDuHSiVGvz9c6h=Pe=aN+FKZOrdNPfbTOk3XV+WFKYQ@mail.gmail.com>
Subject: Re: Row pattern recognition
To: jian he <jian.universality@gmail.com>
Cc: Tatsuo Ishii <ishii@postgresql.org>, zsolt.parragi@percona.com,
 sjjang112233@gmail.com,
	vik@postgresfriends.org, er@xs4all.nl, jacob.champion@enterprisedb.com,
	david.g.johnston@gmail.com, peter@eisentraut.org, li.evan.chao@gmail.com,
	pgsql-hackers@postgresql.org
Content-Type: multipart/alternative; boundary="000000000000d338e506534ca2c0"
Archived-At: 
 <https://www.postgresql.org/message-id/CAAAe_zAZDuHSiVGvz9c6h%3DPe%3DaN%2BFKZOrdNPfbTOk3XV%2BWFKYQ%40mail.gmail.com>
Precedence: bulk

--000000000000d338e506534ca2c0
Content-Type: text/plain; charset="UTF-8"

Subject: Re: Row pattern recognition

Hi Jian,

> 3.
> 240 variables is enough, as each variable supports multiple complex
> AND/OR conditions. [...] reserving the remaining ones in advance is a
> future-proof approach.

Thanks -- that's (3), and Tatsuo landed on the same choice, so it's
settled. I'll make the change for v48.

> XII-4. Memory Pool Management
>   Choice: Custom free list [...]
> It would be better simply to mention that:
> RPRNFAState and RPRNFAContext are allocated in a partition-lifespan
> memory context; they will be destroyed in release_partition.

Agreed -- that's clearer than the rationale list. I'll replace XII-4 with
that wording.

> If ctx->matchedState points to one of the states already in ctx->states,
> will nfa_state_free() be called on the same RPRNFAState twice? Is this
> double-free permitted, or do we have a mechanism in place to guard
> against it?

No -- they're disjoint by construction, so the two frees never touch the
same state. Step by step:

  - nfa_advance() detaches ctx->states up front (sets it to NULL) and
    rebuilds the list from scratch.
  - Each old state is pulled off and advanced; the one that reaches FIN is
    moved into matchedState and never re-added to ctx->states.
  - So at any moment a state is either on ctx->states or it is
    matchedState, never both.
  - ExecRPRFreeContext frees the list and frees matchedState -- disjoint
    sets, no overlap, so no guard is needed.
  - (When a new FIN replaces matchedState, the previous one is freed right
    there.)

> if (currentPos == ctxFrameEnd) {
>     nfa_match(winstate, ctx, NULL);
>     continue;
> }
> If I comment out the CONTINUE, the entire regression still succeeds.

It isn't dead -- it's the N-FOLLOWING boundary handler. nfa_match(ctx,
NULL) forces a mismatch that finalizes the context on its matchedState,
and the continue skips the rest since it's done for this row.

Without the continue, that finalized context is matched a second time in
the same row -- a context-matched-twice error. It only looks harmless
because the forced mismatch already emptied ctx->states, so the second
nfa_match iterates nothing (and it would also re-run
nfa_reevaluate_dependent_vars(), overwriting the shared nfaVarMatched the
other contexts read). So I'd keep it: a finalized context must not be
matched twice in the same row.

Thanks,
Henson

--000000000000d338e506534ca2c0
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr">Subject: Re: Row pattern recognition<br><br>Hi Jian,<br><b=
r>&gt; 3.<br>&gt; 240 variables is enough, as each variable supports multip=
le complex<br>&gt; AND/OR conditions. [...] reserving the remaining ones in=
 advance is a<br>&gt; future-proof approach.<br><br>Thanks -- that&#39;s (3=
), and Tatsuo landed on the same choice, so it&#39;s<br>settled. I&#39;ll m=
ake the change for v48.<br><br>&gt; XII-4. Memory Pool Management<br>&gt; =
=C2=A0 Choice: Custom free list [...]<br>&gt; It would be better simply to =
mention that:<br>&gt; RPRNFAState and RPRNFAContext are allocated in a part=
ition-lifespan<br>&gt; memory context; they will be destroyed in release_pa=
rtition.<br><br>Agreed -- that&#39;s clearer than the rationale list. I&#39=
;ll replace XII-4 with<br>that wording.<br><br>&gt; If ctx-&gt;matchedState=
 points to one of the states already in ctx-&gt;states,<br>&gt; will nfa_st=
ate_free() be called on the same RPRNFAState twice? Is this<br>&gt; double-=
free permitted, or do we have a mechanism in place to guard<br>&gt; against=
 it?<br><br>No -- they&#39;re disjoint by construction, so the two frees ne=
ver touch the<br>same state. Step by step:<br><br>=C2=A0 - nfa_advance() de=
taches ctx-&gt;states up front (sets it to NULL) and<br>=C2=A0 =C2=A0 rebui=
lds the list from scratch.<br>=C2=A0 - Each old state is pulled off and adv=
anced; the one that reaches FIN is<br>=C2=A0 =C2=A0 moved into matchedState=
 and never re-added to ctx-&gt;states.<br>=C2=A0 - So at any moment a state=
 is either on ctx-&gt;states or it is<br>=C2=A0 =C2=A0 matchedState, never =
both.<br>=C2=A0 - ExecRPRFreeContext frees the list and frees matchedState =
-- disjoint<br>=C2=A0 =C2=A0 sets, no overlap, so no guard is needed.<br>=
=C2=A0 - (When a new FIN replaces matchedState, the previous one is freed r=
ight<br>=C2=A0 =C2=A0 there.)<br><br>&gt; if (currentPos =3D=3D ctxFrameEnd=
) {<br>&gt; =C2=A0 =C2=A0 nfa_match(winstate, ctx, NULL);<br>&gt; =C2=A0 =
=C2=A0 continue;<br>&gt; }<br>&gt; If I comment out the CONTINUE, the entir=
e regression still succeeds.<br><br>It isn&#39;t dead -- it&#39;s the N-FOL=
LOWING boundary handler. nfa_match(ctx,<br>NULL) forces a mismatch that fin=
alizes the context on its matchedState,<br>and the continue skips the rest =
since it&#39;s done for this row.<br><br>Without the continue, that finaliz=
ed context is matched a second time in<br>the same row -- a context-matched=
-twice error. It only looks harmless<br>because the forced mismatch already=
 emptied ctx-&gt;states, so the second<br>nfa_match iterates nothing (and i=
t would also re-run<br>nfa_reevaluate_dependent_vars(), overwriting the sha=
red nfaVarMatched the<br>other contexts read). So I&#39;d keep it: a finali=
zed context must not be<br>matched twice in the same row.<br><br>Thanks,<br=
>Henson<br></div>

--000000000000d338e506534ca2c0--