Re: Row pattern recognition

Hi.

Below is the reply for the message
https://www.postgresql.org/message-id/CAAAe_zDWbJUJWjfGBCp8eCyvaJcdr3jjXkAke8toVEd_zd8MUA%40mail.gmail.com
------------------------------------------------------------
+ * Free the previous predicate evaluation's storage. A DEFINE
+ * predicate leaves nothing behind but the RPRVarMatch stored below --
+ * the navigation steps stabilize pass-by-ref results in this same
+ * context, and those are consumed before the predicate returns -- so
+ * resetting here is always safe and no caller has to arrange it.
*/
+ ResetExprContext(winstate->rprContext);
+
result = ExecEvalExprSwitchContext(exprState, winstate->rprContext,
&isnull);

The above change looks good to me.
No comments should be necessary for ResetExprContext or
ExecEvalExprSwitchContext.
This is because they are common routines.

2002 Evaluate an RPR DEFINE clause in the context reserved for [leak]
2019 Reset the DEFINE evaluation context where the predicate [cleanup]
2002 can be merged into 2019.

> 2010 Reject whole-row references and qualified names in a DEFINE
https://www.postgresql.org/message-id/attachment/203224/nocfbot-2010-reject-whole-row-define.txt

src/test/regress/sql/rpr.sql | 393 ++++++++++++-
src/test/regress/sql/rpr_base.sql | 24 +
src/test/regress/sql/rpr_integration.sql | 28 +-

For wholerow rejection, one or two test queries should be enough, otherwise
people may find it overwhelming and hard to review.

Even if we fail to test some corner cases, they aren't that closely
related to RPR, so we can always add more regression test cases later.

@@ -1986,8 +2035,16 @@ transformSubLink(ParseState *pstate, SubLink *sublink)
* are doable with the existing infrastructure -- they are
* left as future work, not blocked on any other feature.
* Until then this blanket rejection is intentional
- * over-rejection, not a standard fit; it subsumes both (a)
- * and (b) by making the subquery itself unreachable.
+ * over-rejection, not a standard fit.
+ *
+ * It rejects the SubLink, which is not the same as keeping
+ * the subquery unanalyzed: a construct that analyzes its
+ * query before building the SubLink, as
+ * transformJsonArrayQueryConstructor() does, has already
+ * resolved names and opened relations inside it by the time
+ * we get here, and reports its own errors first. Whoever
+ * implements (a) and (b) must not read this rejection as
+ * proof that nothing inside a DEFINE subquery runs.

transformSubLink. too much comments for (case EXPR_KIND_RPR_DEFINE).
The following should suffice:
(The standard allows certain subqueries in a DEFINE clause. To support
it you may also look at transformJsonArrayQueryConstructor(), which
analyzes its query before the SubLink reaches this check.
FIXME someday.)

Actually, we don't need to change transformWholeRowRef at all.
See the atached no_need_change_transformWholeRowRef.txt, of course,
this is based on your nocfbot-2010-reject-whole-row-define.txt

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