Re: Row pattern recognition

Hi Henson,

> Hi Tatsuo,
> 
> I reviewed the planner's Window clause optimizations and found
> additional issues with RPR windows.  Thanks to SungJun's Oracle
> cross-validation work, all converted regression tests now pass
> on both systems.  With these fixes, I'd like to move on to
> non-functional quality testing next.

Sounds good.

> Here are eight incremental patches on top of v44.
> 
> 
> nocfbot-0001: Fix elog message to use lowercase per PostgreSQL convention
> 
>   Minor style fix: lowercase the elog error message in
>   nodeWindowAgg.c to follow PostgreSQL coding convention.

Thanks.

> nocfbot-0002: Fix RPR reluctant quantifier flag lost during VIEW
> serialization
> 
>   The reluctant flag on quantifiers was lost when a VIEW containing
>   RPR was serialized and restored via ruleutils.c.  This patch fixes
>   gram.y, rpr.c, ruleutils.c, and parsenodes.h so the flag survives
>   the round-trip.

Great.

> nocfbot-0003: Expand RPR test coverage and improve test comments
> 
>   Reorganizes test cases across rpr.sql, rpr_base.sql, rpr_explain.sql,
>   and rpr_nfa.sql.  Moves base functionality tests from rpr.sql to
>   rpr_base.sql and NFA-specific tests to rpr_nfa.sql.  Adds better
>   section comments throughout.  Duplicate tests in rpr.sql were
>   removed.  No reduction in test coverage.

Sounds good.

> nocfbot-0004: Keep RPR test objects for pg_upgrade/pg_dump testing
> 
>   Adjusts rpr_base.sql and rpr_explain.sql so that test tables and
>   views are not dropped at the end of the test.  This allows
>   pg_upgrade and pg_dump regression testing to cover RPR objects.

Great.

> nocfbot-0005: Disable run condition pushdown for RPR windows
> 
>   Revised version of your run condition fix [1].  Existing test
>   expected output updated accordingly.

Thanks for including the patch.

> nocfbot-0006: Disable frame optimization for RPR windows
> 
>   Prevents the planner from optimizing the window frame for RPR
>   windows.  The frame clause in RPR has different semantics (it
>   bounds the pattern search space), so standard frame optimizations
>   must be disabled.  Adds EXPLAIN tests to verify.  Please review
>   this one -- it's a newly discovered issue.

Oh, this is a very important finding! I will definitely look into the
patch.

> nocfbot-0007: Add stock scenario tests for RPR pattern matching
> 
>   Adds stock trading scenario tests using realistic synthetic data
>   (stock.data with 1632 rows).  Tests V-shape recovery, W-shape,
>   consecutive rises, and other common pattern matching use cases.

Excellent.

> nocfbot-0008: Fix zero-min reluctant quantifier to produce zero-length match
> 
>   Fixes the bug reported by SungJun [2]: reluctant quantifiers with
>   min=0 (A*?, A??) were incorrectly consuming at least one row instead
>   of producing a zero-length match.  The NFA can now internally
>   distinguish between a zero-length match and no match, so it will
>   be ready when additional infrastructure (e.g. MATCH_NUMBER) is
>   added.  Now matches Oracle behavior.

Does "a zero-length match" mean "an empty match"?

BTW, currently we place all nfa_* functions at the bottom of
nodeWindowAgg.c.  However nodeWindowAgg.c in master branch places "API
exposed to window functions" at the bottom of the file. Do you think
we should follow the way?

Best regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp