Re: Row pattern recognition

Hi.

This thread has high traffic, so my review is still based on
https://github.com/assam258-5892/postgres/commits/RPR

/*
 * nfa_eval_var_match
 *
 * Evaluate if a VAR element matches the current row.
 *
 * varMatched is a pre-evaluated boolean array indexed by varId, computed
 * once per row by evaluating all DEFINE expressions.  NULL means no DEFINE
 * clauses exist (only possible during early development/testing).
 *
 * Per ISO/IEC 19075-5 Feature R020, pattern variables not listed in DEFINE
 * are implicitly TRUE -- they match every row.  This is checked via
 * varId >= list_length.
 */

``(only possible during early development/testing).`` comment is not necessary?

nfa_match desperately needs an extra i(nt64 currentPos) argument. The
whole thing execRPR.c is
fairly complicated, and adding currentPos makes it much easier to understand the
current execution state when debugging with GDB.


    /*
     * Evaluate VAR elements against current row. For VARs that reach max
     * count with END next, advance through the chain of END elements inline
     * so absorb phase can compare states at comparison points.
     */
The comments above in nfa_match are unnecessary because in ``if
(RPRElemIsAbsorbableBranch(elem)``
we already have many comments, and the meaning seems the same.

Similar to REG_DEBUG, we really need an RPR_DEBUG option. For example, in
nfa_match, it would be useful to verify that ctx->states is not a circular
linked list. When RPR_DEBUG is enabled, we can perform these kinds of sanity
checks and other debug-only validations without affecting release builds.

   <para>
    The SQL standard defines more subclauses: <literal>MEASURES</literal>
    and <literal>SUBSET</literal>. They are not currently supported
    in <productname>PostgreSQL</productname>. Also in the standard there are
    more variations in <literal>AFTER MATCH</literal> clause.
   </para>
This part should stay in the Compatibility section.



--
jian
https://www.enterprisedb.com/