Re: Row pattern recognition

From: Henson Choi <assam258(at)gmail(dot)com>
To: jian he <jian(dot)universality(at)gmail(dot)com>
Cc: Tatsuo Ishii <ishii(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, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Row pattern recognition
Date: 2026-06-02 22:14:21
Message-ID: CAAAe_zAZDuHSiVGvz9c6h=Pe=aN+FKZOrdNPfbTOk3XV+WFKYQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2026-06-02 22:23:16 Re: injection_points: Switch wait/wakeup to use atomics rather than latches
Previous Message Richard Guo 2026-06-02 22:13:03 Re: Wrong unsafe-flag test in check_output_expressions()