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/
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 24099beb0d..40d5de0a32 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -72,8 +72,7 @@ static Node *transformXmlSerialize(ParseState *pstate,
XmlSerialize *xs);
static Node *transformBooleanTest(ParseState *pstate, BooleanTest *b);
static Node *transformCurrentOfExpr(ParseState *pstate, CurrentOfExpr *cexpr);
static Node *transformColumnRef(ParseState *pstate, ColumnRef *cref);
-static Node *transformWholeRowRef(ParseState *pstate, bool for_func_call,
-
ParseNamespaceItem *nsitem,
+static Node *transformWholeRowRef(ParseState *pstate, ParseNamespaceItem
*nsitem,
int
sublevels_up, int location);
static Node *transformIndirection(ParseState *pstate, A_Indirection *ind);
static Node *transformTypeCast(ParseState *pstate, TypeCast *tc);
@@ -513,7 +512,7 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
char *nspname = NULL;
char *relname = NULL;
char *colname = NULL;
- ParseNamespaceItem *nsitem;
+ ParseNamespaceItem *nsitem = NULL;
int levels_up;
enum
{
@@ -709,8 +708,8 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
cref->location,
&levels_up);
if (nsitem)
- node =
transformWholeRowRef(pstate, false, nsitem,
-
levels_up, cref->location);
+ node =
transformWholeRowRef(pstate, nsitem, levels_up,
+
cref->location);
}
break;
}
@@ -734,8 +733,8 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
/* Whole-row reference? */
if (IsA(field2, A_Star))
{
- node = transformWholeRowRef(pstate,
false, nsitem,
-
levels_up, cref->location);
+ node = transformWholeRowRef(pstate,
nsitem, levels_up,
+
cref->location);
break;
}
@@ -747,8 +746,8 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
if (node == NULL)
{
/* Try it as a function call on the
whole row */
- node = transformWholeRowRef(pstate,
true, nsitem,
-
levels_up, cref->location);
+ node = transformWholeRowRef(pstate,
nsitem, levels_up,
+
cref->location);
node = ParseFuncOrColumn(pstate,
list_make1(makeString(colname)),
list_make1(node),
@@ -781,8 +780,8 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
/* Whole-row reference? */
if (IsA(field3, A_Star))
{
- node = transformWholeRowRef(pstate,
false, nsitem,
-
levels_up, cref->location);
+ node = transformWholeRowRef(pstate,
nsitem, levels_up,
+
cref->location);
break;
}
@@ -794,8 +793,8 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
if (node == NULL)
{
/* Try it as a function call on the
whole row */
- node = transformWholeRowRef(pstate,
true, nsitem,
-
levels_up, cref->location);
+ node = transformWholeRowRef(pstate,
nsitem, levels_up,
+
cref->location);
node = ParseFuncOrColumn(pstate,
list_make1(makeString(colname)),
list_make1(node),
@@ -840,8 +839,8 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
/* Whole-row reference? */
if (IsA(field4, A_Star))
{
- node = transformWholeRowRef(pstate,
false, nsitem,
-
levels_up, cref->location);
+ node = transformWholeRowRef(pstate,
nsitem, levels_up,
+
cref->location);
break;
}
@@ -853,8 +852,8 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
if (node == NULL)
{
/* Try it as a function call on the
whole row */
- node = transformWholeRowRef(pstate,
true, nsitem,
-
levels_up, cref->location);
+ node = transformWholeRowRef(pstate,
nsitem, levels_up,
+
cref->location);
node = ParseFuncOrColumn(pstate,
list_make1(makeString(colname)),
list_make1(node),
@@ -870,6 +869,16 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
break;
}
+ if (pstate->p_expr_kind == EXPR_KIND_RPR_DEFINE && node != NULL)
+ {
+ if (IsA(llast(cref->fields), A_Star) ||
(list_length(cref->fields) == 1 && nsitem != NULL))
+ ereport(ERROR,
+ errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("whole-row reference is not
allowed in DEFINE clause"),
+ errhint("A DEFINE condition may
reference individual columns only."),
+ parser_errposition(pstate,
cref->location));
+ }
+
/*
* Now give the PostParseColumnRefHook, if any, a chance. We pass the
* translation-so-far so that it can throw an error if it wishes in the
@@ -2792,29 +2801,11 @@ transformCurrentOfExpr(ParseState *pstate,
CurrentOfExpr *cexpr)
/*
* Construct a whole-row reference to represent the notation "relation.*".
- *
- * for_func_call is true when transformColumnRef is building the reference
- * speculatively, to retry a name that did not resolve as a column as a
- * function call on the composite value. The query does not contain a
- * whole-row reference in that case, so restrictions on writing one must not
- * fire; whatever the retry resolves to is diagnosed by the caller.
*/
static Node *
-transformWholeRowRef(ParseState *pstate, bool for_func_call,
- ParseNamespaceItem *nsitem, int
sublevels_up,
- int location)
+transformWholeRowRef(ParseState *pstate, ParseNamespaceItem *nsitem,
+ int sublevels_up, int location)
{
- /*
- * A DEFINE clause cannot use a whole-row reference: ISO/IEC 19075-5 6.5
- * limits the range variables in scope to the row pattern variables.
- */
- if (pstate->p_expr_kind == EXPR_KIND_RPR_DEFINE && !for_func_call)
- ereport(ERROR,
- errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("whole-row reference is not allowed in
DEFINE clause"),
- errhint("A DEFINE condition may reference
individual columns only."),
- parser_errposition(pstate, location));
-
/*
* Build the appropriate referencing node. Normally this can be a
* whole-row Var, but if the nsitem is a JOIN USING alias then it
contains
