| 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
| 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() |