public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v1 2/5] Changing SPI_connect and SPI_connect_ext to return void.
3+ messages / 3 participants
[nested] [flat]

* [PATCH v1 2/5] Changing SPI_connect and SPI_connect_ext to return void.
@ 2019-11-07 16:29  Mark Dilger <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: Mark Dilger @ 2019-11-07 16:29 UTC (permalink / raw)

SPI_connect and SPI_connect_ext were always returning SPI_OK_CONNECT if they
returned at all.  Numerous places in the code were needlessly checking the
return value of these two functions.  I am changing the functions to return
void, and including two backward compatibility macros to avoid breaking
third-party code which either does not get updated, or which needs to work
across different versions.

This patch is by me, but some of these ideas and a sketch of the backward
compatibility macros provided by Tom Lane:

https://www.postgresql.org/message-id/14823.1558630129%40sss.pgh.pa.us
---
 contrib/dblink/dblink.c                       |  4 +---
 contrib/spi/refint.c                          |  8 ++-----
 contrib/tablefunc/tablefunc.c                 | 17 ++++---------
 src/backend/commands/matview.c                |  3 +--
 src/backend/executor/spi.c                    | 11 +++++----
 src/backend/utils/adt/ri_triggers.c           | 24 +++++++------------
 src/backend/utils/adt/ruleutils.c             |  6 ++---
 src/include/executor/spi.h                    | 18 ++++++++++++--
 src/pl/plpgsql/src/pl_handler.c               |  9 +++----
 .../modules/test_predtest/test_predtest.c     |  3 +--
 src/test/regress/regress.c                    |  3 +--
 11 files changed, 45 insertions(+), 61 deletions(-)

diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 7e225589a9..5587c57bf9 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -2379,9 +2379,7 @@ get_tuple_of_interest(Relation rel, int *pkattnums, int pknumatts, char **src_pk
 	/*
 	 * Connect to SPI manager
 	 */
-	if ((ret = SPI_connect()) < 0)
-		/* internal error */
-		elog(ERROR, "SPI connect failure - returned %d", ret);
+	SPI_connect();
 
 	initStringInfo(&buf);
 
diff --git a/contrib/spi/refint.c b/contrib/spi/refint.c
index 00253a63e9..a71ef931ec 100644
--- a/contrib/spi/refint.c
+++ b/contrib/spi/refint.c
@@ -107,9 +107,7 @@ check_primary_key(PG_FUNCTION_ARGS)
 	tupdesc = rel->rd_att;
 
 	/* Connect to SPI manager */
-	if ((ret = SPI_connect()) < 0)
-		/* internal error */
-		elog(ERROR, "check_primary_key: SPI_connect returned %d", ret);
+	SPI_connect();
 
 	/*
 	 * We use SPI plan preparation feature, so allocate space to place key
@@ -326,9 +324,7 @@ check_foreign_key(PG_FUNCTION_ARGS)
 	tupdesc = rel->rd_att;
 
 	/* Connect to SPI manager */
-	if ((ret = SPI_connect()) < 0)
-		/* internal error */
-		elog(ERROR, "check_foreign_key: SPI_connect returned %d", ret);
+	SPI_connect();
 
 	/*
 	 * We use SPI plan preparation feature, so allocate space to place key
diff --git a/contrib/tablefunc/tablefunc.c b/contrib/tablefunc/tablefunc.c
index 256d52fc62..8866bbf0ed 100644
--- a/contrib/tablefunc/tablefunc.c
+++ b/contrib/tablefunc/tablefunc.c
@@ -379,9 +379,7 @@ crosstab(PG_FUNCTION_ARGS)
 	per_query_ctx = rsinfo->econtext->ecxt_per_query_memory;
 
 	/* Connect to SPI manager */
-	if ((ret = SPI_connect()) < 0)
-		/* internal error */
-		elog(ERROR, "crosstab: SPI_connect returned %d", ret);
+	SPI_connect();
 
 	/* Retrieve the desired rows */
 	ret = SPI_execute(sql, true, 0);
@@ -726,9 +724,7 @@ load_categories_hash(char *cats_sql, MemoryContext per_query_ctx)
 								HASH_ELEM | HASH_CONTEXT);
 
 	/* Connect to SPI manager */
-	if ((ret = SPI_connect()) < 0)
-		/* internal error */
-		elog(ERROR, "load_categories_hash: SPI_connect returned %d", ret);
+	SPI_connect();
 
 	/* Retrieve the category name rows */
 	ret = SPI_execute(cats_sql, true, 0);
@@ -805,9 +801,7 @@ get_crosstab_tuplestore(char *sql,
 	tupstore = tuplestore_begin_heap(randomAccess, false, work_mem);
 
 	/* Connect to SPI manager */
-	if ((ret = SPI_connect()) < 0)
-		/* internal error */
-		elog(ERROR, "get_crosstab_tuplestore: SPI_connect returned %d", ret);
+	SPI_connect();
 
 	/* Now retrieve the crosstab source rows */
 	ret = SPI_execute(sql, true, 0);
@@ -1157,15 +1151,12 @@ connectby(char *relname,
 		  AttInMetadata *attinmeta)
 {
 	Tuplestorestate *tupstore = NULL;
-	int			ret;
 	MemoryContext oldcontext;
 
 	int			serial = 1;
 
 	/* Connect to SPI manager */
-	if ((ret = SPI_connect()) < 0)
-		/* internal error */
-		elog(ERROR, "connectby: SPI_connect returned %d", ret);
+	SPI_connect();
 
 	/* switch to longer term context to create the tuple store */
 	oldcontext = MemoryContextSwitchTo(per_query_ctx);
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index 537d0e8cef..b11c98a093 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -605,8 +605,7 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
 	relnatts = RelationGetNumberOfAttributes(matviewRel);
 
 	/* Open SPI context. */
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect();
 
 	/* Analyze the temp table with the new contents. */
 	appendStringInfo(&querybuf, "ANALYZE %s", tempname);
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 2bb49e6919..c3943ebbb9 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -82,16 +82,19 @@ static MemoryContext _SPI_execmem(void);
 static MemoryContext _SPI_procmem(void);
 static bool _SPI_checktuples(void);
 
+/* Defend against BACKWARDS_COMPATIBLE_SPI_CALLS wrapper macros */
+#undef SPI_connect
+#undef SPI_connect_ext
 
 /* =================== interface functions =================== */
 
-int
+void
 SPI_connect(void)
 {
-	return SPI_connect_ext(0);
+	SPI_connect_ext(0);
 }
 
-int
+void
 SPI_connect_ext(int options)
 {
 	int			newdepth;
@@ -168,8 +171,6 @@ SPI_connect_ext(int options)
 	SPI_processed = 0;
 	SPI_tuptable = NULL;
 	SPI_result = 0;
-
-	return SPI_OK_CONNECT;
 }
 
 int
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index caf27f8bcb..1462f29e25 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -334,8 +334,7 @@ RI_FKey_check(TriggerData *trigdata)
 			break;
 	}
 
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect();
 
 	/* Fetch or prepare a saved plan for the real check */
 	ri_BuildQueryKey(&qkey, riinfo, RI_PLAN_CHECK_LOOKUPPK);
@@ -459,8 +458,7 @@ ri_Check_Pk_Match(Relation pk_rel, Relation fk_rel,
 	/* Only called for non-null rows */
 	Assert(ri_NullCheck(RelationGetDescr(pk_rel), oldslot, riinfo, true) == RI_KEYS_NONE_NULL);
 
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect();
 
 	/*
 	 * Fetch or prepare a saved plan for checking PK table with values coming
@@ -646,8 +644,7 @@ ri_restrict(TriggerData *trigdata, bool is_no_action)
 		return PointerGetDatum(NULL);
 	}
 
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect();
 
 	/*
 	 * Fetch or prepare a saved plan for the restrict lookup (it's the same
@@ -756,8 +753,7 @@ RI_FKey_cascade_del(PG_FUNCTION_ARGS)
 	pk_rel = trigdata->tg_relation;
 	oldslot = trigdata->tg_trigslot;
 
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect();
 
 	/* Fetch or prepare a saved plan for the cascaded delete */
 	ri_BuildQueryKey(&qkey, riinfo, RI_PLAN_CASCADE_DEL_DODELETE);
@@ -865,8 +861,7 @@ RI_FKey_cascade_upd(PG_FUNCTION_ARGS)
 	newslot = trigdata->tg_newslot;
 	oldslot = trigdata->tg_trigslot;
 
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect();
 
 	/* Fetch or prepare a saved plan for the cascaded update */
 	ri_BuildQueryKey(&qkey, riinfo, RI_PLAN_CASCADE_UPD_DOUPDATE);
@@ -1040,8 +1035,7 @@ ri_set(TriggerData *trigdata, bool is_set_null)
 	pk_rel = trigdata->tg_relation;
 	oldslot = trigdata->tg_trigslot;
 
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect();
 
 	/*
 	 * Fetch or prepare a saved plan for the set null/default operation (it's
@@ -1464,8 +1458,7 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
 							 PGC_USERSET, PGC_S_SESSION,
 							 GUC_ACTION_SAVE, true, 0, false);
 
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect();
 
 	/*
 	 * Generate the plan.  We don't need to cache it, and there are no
@@ -1699,8 +1692,7 @@ RI_PartitionRemove_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
 							 PGC_USERSET, PGC_S_SESSION,
 							 GUC_ACTION_SAVE, true, 0, false);
 
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect();
 
 	/*
 	 * Generate the plan.  We don't need to cache it, and there are no
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 3e64390d81..acf3f804af 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -534,8 +534,7 @@ pg_get_ruledef_worker(Oid ruleoid, int prettyFlags)
 	/*
 	 * Connect to SPI manager
 	 */
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect();
 
 	/*
 	 * On the first call prepare the plan to lookup pg_rewrite. We read
@@ -727,8 +726,7 @@ pg_get_viewdef_worker(Oid viewoid, int prettyFlags, int wrapColumn)
 	/*
 	 * Connect to SPI manager
 	 */
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect();
 
 	/*
 	 * On the first call prepare the plan to lookup pg_rewrite. We read
diff --git a/src/include/executor/spi.h b/src/include/executor/spi.h
index a2ea6dda5f..2d505610ff 100644
--- a/src/include/executor/spi.h
+++ b/src/include/executor/spi.h
@@ -81,8 +81,8 @@ extern PGDLLIMPORT uint64 SPI_processed;
 extern PGDLLIMPORT SPITupleTable *SPI_tuptable;
 extern PGDLLIMPORT int SPI_result;
 
-extern int	SPI_connect(void);
-extern int	SPI_connect_ext(int options);
+extern void	SPI_connect(void);
+extern void	SPI_connect_ext(int options);
 extern int	SPI_finish(void);
 extern int	SPI_execute(const char *src, bool read_only, long tcount);
 extern int	SPI_execute_plan(SPIPlanPtr plan, Datum *Values, const char *Nulls,
@@ -172,4 +172,18 @@ extern void AtEOXact_SPI(bool isCommit);
 extern void AtEOSubXact_SPI(bool isCommit, SubTransactionId mySubid);
 extern bool SPI_inside_nonatomic_context(void);
 
+/*
+ * Backward compatibility for third-party code which expects the older SPI
+ * function prototypes returning int rather than void.
+ *
+ * This switch should only be defined when compiling legacy third-party code
+ * which expects an integer return value, otherwise, at least under some
+ * compilers, compilation may draw warnings about the right-hand side value
+ * being ignored (-Wunused-value).
+ */
+#ifdef BACKWARDS_COMPATIBLE_SPI_CALLS
+#define SPI_connect() (SPI_connect(), SPI_OK_CONNECT)
+#define SPI_connect_ext(o) (SPI_connect_ext(o), SPI_OK_CONNECT)
+#endif
+
 #endif							/* SPI_H */
diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handler.c
index 1b592c8a52..3e41f36f61 100644
--- a/src/pl/plpgsql/src/pl_handler.c
+++ b/src/pl/plpgsql/src/pl_handler.c
@@ -234,8 +234,7 @@ plpgsql_call_handler(PG_FUNCTION_ARGS)
 	/*
 	 * Connect to SPI manager
 	 */
-	if ((rc = SPI_connect_ext(nonatomic ? SPI_OPT_NONATOMIC : 0)) != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed: %s", SPI_result_code_string(rc));
+	SPI_connect_ext(nonatomic ? SPI_OPT_NONATOMIC : 0);
 
 	/* Find or compile the function */
 	func = plpgsql_compile(fcinfo, false);
@@ -303,8 +302,7 @@ plpgsql_inline_handler(PG_FUNCTION_ARGS)
 	/*
 	 * Connect to SPI manager
 	 */
-	if ((rc = SPI_connect_ext(codeblock->atomic ? 0 : SPI_OPT_NONATOMIC)) != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed: %s", SPI_result_code_string(rc));
+	SPI_connect_ext(codeblock->atomic ? 0 : SPI_OPT_NONATOMIC);
 
 	/* Compile the anonymous code block */
 	func = plpgsql_compile_inline(codeblock->source_text);
@@ -468,8 +466,7 @@ plpgsql_validator(PG_FUNCTION_ARGS)
 		/*
 		 * Connect to SPI manager (is this needed for compilation?)
 		 */
-		if ((rc = SPI_connect()) != SPI_OK_CONNECT)
-			elog(ERROR, "SPI_connect failed: %s", SPI_result_code_string(rc));
+		SPI_connect();
 
 		/*
 		 * Set up a fake fcinfo with just enough info to satisfy
diff --git a/src/test/modules/test_predtest/test_predtest.c b/src/test/modules/test_predtest/test_predtest.c
index 10cd4f9ae5..d612e29198 100644
--- a/src/test/modules/test_predtest/test_predtest.c
+++ b/src/test/modules/test_predtest/test_predtest.c
@@ -54,8 +54,7 @@ test_predtest(PG_FUNCTION_ARGS)
 	int			i;
 
 	/* We use SPI to parse, plan, and execute the test query */
-	if (SPI_connect() != SPI_OK_CONNECT)
-		elog(ERROR, "SPI_connect failed");
+	SPI_connect();
 
 	/*
 	 * First, plan and execute the query, and inspect the results.  To the
diff --git a/src/test/regress/regress.c b/src/test/regress/regress.c
index 2f66d76ab3..7d8086cd2c 100644
--- a/src/test/regress/regress.c
+++ b/src/test/regress/regress.c
@@ -368,8 +368,7 @@ ttdummy(PG_FUNCTION_ARGS)
 	newoff = Int32GetDatum((int32) DatumGetInt64(newoff));
 
 	/* Connect to SPI manager */
-	if ((ret = SPI_connect()) < 0)
-		elog(ERROR, "ttdummy (%s): SPI_connect returned %d", relname, ret);
+	SPI_connect();
 
 	/* Fetch tuple values and nulls */
 	cvals = (Datum *) palloc(natts * sizeof(Datum));
-- 
2.20.1


--------------EE32DA189D040DF51B4A06B1
Content-Type: text/x-patch;
 name="v1-0003-Refactoring-SPI-trigger-related-errors.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="v1-0003-Refactoring-SPI-trigger-related-errors.patch"



^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* [PATCH v1 2/7] Row pattern recognition patch (parse/analysis).
@ 2023-06-25 11:48  Tatsuo Ishii <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: Tatsuo Ishii @ 2023-06-25 11:48 UTC (permalink / raw)

---
 src/backend/parser/parse_agg.c    |   7 ++
 src/backend/parser/parse_clause.c | 196 +++++++++++++++++++++++++++++-
 src/backend/parser/parse_expr.c   |   4 +
 src/backend/parser/parse_func.c   |   3 +
 4 files changed, 209 insertions(+), 1 deletion(-)

diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c
index 85cd47b7ae..aa7a1cee80 100644
--- a/src/backend/parser/parse_agg.c
+++ b/src/backend/parser/parse_agg.c
@@ -564,6 +564,10 @@ check_agglevels_and_constraints(ParseState *pstate, Node *expr)
 			errkind = true;
 			break;
 
+		case EXPR_KIND_RPR_DEFINE:
+			errkind = true;
+			break;
+
 			/*
 			 * There is intentionally no default: case here, so that the
 			 * compiler will warn if we add a new ParseExprKind without
@@ -953,6 +957,9 @@ transformWindowFuncCall(ParseState *pstate, WindowFunc *wfunc,
 		case EXPR_KIND_CYCLE_MARK:
 			errkind = true;
 			break;
+		case EXPR_KIND_RPR_DEFINE:
+			errkind = true;
+			break;
 
 			/*
 			 * There is intentionally no default: case here, so that the
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index f61f794755..5bb11add3c 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -100,7 +100,10 @@ static WindowClause *findWindowClause(List *wclist, const char *name);
 static Node *transformFrameOffset(ParseState *pstate, int frameOptions,
 								  Oid rangeopfamily, Oid rangeopcintype, Oid *inRangeFunc,
 								  Node *clause);
-
+static void  transformRPR(ParseState *pstate, WindowClause *wc, WindowDef *windef);
+static List *transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef);
+static List *transformPatternClause(ParseState *pstate, WindowClause *wc, WindowDef *windef);
+static List *transformMeasureClause(ParseState *pstate, WindowClause *wc, WindowDef *windef);
 
 /*
  * transformFromClause -
@@ -2949,6 +2952,10 @@ transformWindowDefinitions(ParseState *pstate,
 											 rangeopfamily, rangeopcintype,
 											 &wc->endInRangeFunc,
 											 windef->endOffset);
+
+		/* Process Row Pattern Recognition related clauses */
+		transformRPR(pstate, wc, windef);
+
 		wc->runCondition = NIL;
 		wc->winref = winref;
 
@@ -3814,3 +3821,190 @@ transformFrameOffset(ParseState *pstate, int frameOptions,
 
 	return node;
 }
+
+/*
+ * transformRPR
+ *		Process Row Pattern Recognition related clauses
+ */
+static void
+transformRPR(ParseState *pstate, WindowClause *wc, WindowDef *windef)
+{
+	/* Check Frame option. Frame must start at current row */
+
+	/*
+	 * Window definition exists?
+	 */
+	if (windef == NULL)
+		return;
+
+	/*
+	 * Row Pattern Common Syntax clause exists?
+	 */
+	if (windef->rpCommonSyntax == NULL)
+		return;
+
+	if ((wc->frameOptions & FRAMEOPTION_START_CURRENT_ROW) == 0)
+		ereport(ERROR,
+				(errcode(ERRCODE_SYNTAX_ERROR),
+				 errmsg("FRAME must start at current row when row patttern recognition is used")));
+
+	/* Transform AFTER MACH SKIP TO clause */
+	wc->rpSkipTo = windef->rpCommonSyntax->rpSkipTo;
+	if (wc->rpSkipTo != ST_NEXT_ROW)
+		ereport(ERROR,
+				(errcode(ERRCODE_SYNTAX_ERROR),
+				 errmsg("only AFTER MATCH SKIP TO NEXT_ROW is supported")));
+
+	/* Transform SEEK or INITIAL clause */
+	wc->initial = windef->rpCommonSyntax->initial;
+
+	/* Transform DEFINE clause into list of TargetEntry's */
+	wc->defineClause = transformDefineClause(pstate, wc, windef);
+
+	/* Check PATTERN clause and copy to patternClause */
+	transformPatternClause(pstate, wc, windef);
+
+	/* Transform MEASURE clause */
+	transformMeasureClause(pstate, wc, windef);
+}
+
+/*
+ * transformDefineClause Process DEFINE clause and transform ResTarget into
+ *		list of TargetEntry.
+ *
+ * XXX we only support column reference in row pattern definition search
+ * condition, e.g. "price". <row pattern definition variable name>.<column
+ * reference> is not supported, e.g. "A.price".
+ */
+static List *
+transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef)
+{
+	ListCell	*lc;
+	ResTarget	*restarget, *r;
+	List		*restargets;
+
+
+	/*
+	 * If Row Definition Common Syntax exists, DEFINE clause must exist.
+	 * (the raw parser should have already checked it.)
+	 */
+	Assert(windef->rpCommonSyntax->rpDefs != NULL);
+
+	/*
+	 * Check for duplicate row pattern definition variables.  The standard
+	 * requires that no two row pattern definition variable names shall be
+	 * equivalent.
+	 */
+	restargets = NIL;
+	foreach(lc, windef->rpCommonSyntax->rpDefs)
+	{
+		char	*name;
+		ListCell	*l;
+
+		restarget = (ResTarget *)lfirst(lc);
+		name = restarget->name;
+
+		/*
+		 * Make sure that row pattern definition search condition is a boolean
+		 * expression.
+		 */
+		transformWhereClause(pstate, restarget->val,
+							 EXPR_KIND_RPR_DEFINE, "DEFINE");
+
+		foreach(l, restargets)
+		{
+			char		*n;
+
+			r = (ResTarget *) lfirst(l);
+			n = r->name;
+
+			if (!strcmp(n, name))
+				ereport(ERROR,
+						(errcode(ERRCODE_SYNTAX_ERROR),
+						 errmsg("row pattern definition variable name \"%s\" appears more than once in DEFINE clause",
+								name),
+						 parser_errposition(pstate, exprLocation((Node *)r))));
+		}
+		restargets = lappend(restargets, restarget);
+	}
+	list_free(restargets);
+
+	return transformTargetList(pstate, windef->rpCommonSyntax->rpDefs,
+							   EXPR_KIND_RPR_DEFINE);
+}
+
+/*
+ * transformPatternClause
+ *		Process PATTERN clause and return PATTERN clause in the raw parse tree
+ */
+static List *
+transformPatternClause(ParseState *pstate, WindowClause *wc, WindowDef *windef)
+{
+	List		*patterns;
+	ListCell	*lc, *l;
+
+	/*
+	 * Row Pattern Common Syntax clause exists?
+	 */
+	if (windef->rpCommonSyntax == NULL)
+		return NULL;
+
+	/*
+	 * Primary row pattern variable names in PATTERN clause must appear in
+	 * DEFINE clause as row pattern definition variable names.
+	 */
+	wc->patternVariable = NIL;
+	wc->patternRegexp = NIL;
+	foreach(lc, windef->rpCommonSyntax->rpPatterns)
+	{
+		A_Expr	*a;
+		char	*name;
+		char	*regexp;
+		bool	found = false;
+
+		if (!IsA(lfirst(lc), A_Expr))
+			ereport(ERROR,
+					errmsg("node type is not A_Expr"));
+
+		a = (A_Expr *)lfirst(lc);
+		name = strVal(a->lexpr);
+
+		foreach(l, windef->rpCommonSyntax->rpDefs)
+		{
+			ResTarget	*restarget = (ResTarget *)lfirst(l);
+
+			if (!strcmp(restarget->name, name))
+			{
+				found = true;
+				break;
+			}
+		}
+
+		if (!found)
+			ereport(ERROR,
+					(errcode(ERRCODE_SYNTAX_ERROR),
+					 errmsg("primary row pattern variable name \"%s\" does not appear in DEFINE clause",
+							name),
+					 parser_errposition(pstate, exprLocation((Node *)a))));
+		wc->patternVariable = lappend(wc->patternVariable, makeString(pstrdup(name)));
+		regexp = strVal(lfirst(list_head(a->name)));
+		wc->patternRegexp = lappend(wc->patternRegexp, makeString(pstrdup(regexp)));
+	}
+	return patterns;
+}
+
+/*
+ * transformMeasureClause
+ *		Process MEASURE clause
+ */
+static List *
+transformMeasureClause(ParseState *pstate, WindowClause *wc, WindowDef *windef)
+{
+	if (windef->rowPatternMeasures == NIL)
+		return NIL;
+
+	ereport(ERROR,
+			(errcode(ERRCODE_SYNTAX_ERROR),
+			 errmsg("%s","MEASURE clause is not supported yet"),
+			 parser_errposition(pstate, exprLocation((Node *)windef->rowPatternMeasures))));
+}
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 346fd272b6..20231d9ec0 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -541,6 +541,7 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
 		case EXPR_KIND_COPY_WHERE:
 		case EXPR_KIND_GENERATED_COLUMN:
 		case EXPR_KIND_CYCLE_MARK:
+		case EXPR_KIND_RPR_DEFINE:
 			/* okay */
 			break;
 
@@ -1754,6 +1755,7 @@ transformSubLink(ParseState *pstate, SubLink *sublink)
 		case EXPR_KIND_VALUES:
 		case EXPR_KIND_VALUES_SINGLE:
 		case EXPR_KIND_CYCLE_MARK:
+		case EXPR_KIND_RPR_DEFINE:
 			/* okay */
 			break;
 		case EXPR_KIND_CHECK_CONSTRAINT:
@@ -3133,6 +3135,8 @@ ParseExprKindName(ParseExprKind exprKind)
 			return "GENERATED AS";
 		case EXPR_KIND_CYCLE_MARK:
 			return "CYCLE";
+		case EXPR_KIND_RPR_DEFINE:
+			return "DEFINE";
 
 			/*
 			 * There is intentionally no default: case here, so that the
diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index b3f0b6a137..2ff3699538 100644
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -2656,6 +2656,9 @@ check_srf_call_placement(ParseState *pstate, Node *last_srf, int location)
 		case EXPR_KIND_CYCLE_MARK:
 			errkind = true;
 			break;
+		case EXPR_KIND_RPR_DEFINE:
+			errkind = true;
+			break;
 
 			/*
 			 * There is intentionally no default: case here, so that the
-- 
2.25.1


----Next_Part(Sun_Jun_25_21_05_09_2023_126)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v1-0003-Row-pattern-recognition-patch-planner.patch"



^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: Proposals for EXPLAIN: rename ANALYZE to EXECUTE and extend VERBOSE
@ 2024-11-27 03:19  Tom Lane <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: Tom Lane @ 2024-11-27 03:19 UTC (permalink / raw)
  To: David Rowley <[email protected]>; +Cc: Robert Haas <[email protected]>; Guillaume Lelarge <[email protected]>; Greg Sabino Mullane <[email protected]>; Michael Christofides <[email protected]>; David G. Johnston <[email protected]>; Nikolay Samokhvalov <[email protected]>; pgsql-hackers

David Rowley <[email protected]> writes:
> On Tue, 26 Nov 2024 at 12:12, Tom Lane <[email protected]> wrote:
>> I'm kind of -0.5, but I'd not bothered to vote because it was
>> pretty clear what the result was going to be.

> Just so I can properly understand, how much of the -0.5 is "don't like
> the idea" vs "rather make changes to well-known behaviour"?

Well, I'm with Robert in that I've not found the buffer counts to be
all that useful most of the time.  I grant that they're sometimes
useful, but that's not enough to justify them being on by default:
EXPLAIN output is cluttered enough already.  And I'm also not
thrilled with changing longstanding behavior, but that's a
relatively minor point in this case.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 3+ messages in thread


end of thread, other threads:[~2024-11-27 03:19 UTC | newest]

Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-11-07 16:29 [PATCH v1 2/5] Changing SPI_connect and SPI_connect_ext to return void. Mark Dilger <[email protected]>
2023-06-25 11:48 [PATCH v1 2/7] Row pattern recognition patch (parse/analysis). Tatsuo Ishii <[email protected]>
2024-11-27 03:19 Re: Proposals for EXPLAIN: rename ANALYZE to EXECUTE and extend VERBOSE Tom Lane <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox