public inbox for [email protected]  
help / color / mirror / Atom feed
Re: explain analyze rows=%.0f
9+ messages / 5 participants
[nested] [flat]

* Re: explain analyze rows=%.0f
@ 2023-06-08 17:49  Ibrar Ahmed <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: Ibrar Ahmed @ 2023-06-08 17:49 UTC (permalink / raw)
  To: Gregory Stark (as CFM) <[email protected]>; +Cc: Tom Lane <[email protected]>; Robert Haas <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]

On Mon, Mar 20, 2023 at 7:56 PM Gregory Stark (as CFM) <[email protected]>
wrote:

> On Wed, 4 Jan 2023 at 10:05, Ibrar Ahmed <[email protected]> wrote:
> >
> > Thanks, I have modified everything as suggested, except one point
> >
> > > Don't use variable format strings. They're hard to read and they
> > > probably defeat compile-time checks that the arguments match the
> > > format string. You could use a "*" field width instead, ie
> ...
> > > * Another thought is that the non-text formats tend to prize output
> > > consistency over readability, so maybe we should just always use 2
> > > fractional digits there, rather than trying to minimize visible
> changes.
> >
> > In that, we need to adjust a lot of test case outputs.
>
> > > * We need some doc adjustments, surely, to explain what the heck this
> > > means.
>
> That sounds like three points :) But they seem like pretty good
> arguments to me and straightforward if a little tedious to adjust.
>
> This patch was marked Returned with Feedback and then later Waiting on
> Author. And it hasn't had any updates since January. What is the state
> on this feedback? If it's already done we can set the patch to Ready
> for Commit and if not do you disagree with the proposed changes?
>
> If there is a consensus to modify the test cases' output, I am willing to
make the necessary changes and adjust the patch accordingly. However,
if there is a preference to keep the output of certain test cases unchanged,
I can rebase and modify the patch accordingly to accommodate those
preferences.




> It's actually the kind of code cleanup changes I'm reluctant to bump a
> patch for. It's not like a committer can't make these kinds of changes
> when committing. But there are so many patches they're likely to just
> focus on a different patch when there are adjustments like this
> pending.
>
> --
> Gregory Stark
> As Commitfest Manager
>


-- 
Ibrar Ahmed


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

* [PATCH v2 4/7] Row pattern recognition patch (executor).
@ 2023-06-26 08:05  Tatsuo Ishii <[email protected]>
  0 siblings, 0 replies; 9+ messages in thread

From: Tatsuo Ishii @ 2023-06-26 08:05 UTC (permalink / raw)

---
 src/backend/executor/nodeWindowAgg.c | 225 +++++++++++++++++++-
 src/backend/utils/adt/windowfuncs.c  | 302 ++++++++++++++++++++++++++-
 src/include/catalog/pg_proc.dat      |   9 +
 src/include/nodes/execnodes.h        |  13 ++
 src/include/windowapi.h              |   9 +
 5 files changed, 548 insertions(+), 10 deletions(-)

diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c
index 310ac23e3a..bef2bc62b2 100644
--- a/src/backend/executor/nodeWindowAgg.c
+++ b/src/backend/executor/nodeWindowAgg.c
@@ -48,6 +48,7 @@
 #include "utils/acl.h"
 #include "utils/builtins.h"
 #include "utils/datum.h"
+#include "utils/fmgroids.h"
 #include "utils/expandeddatum.h"
 #include "utils/lsyscache.h"
 #include "utils/memutils.h"
@@ -159,6 +160,14 @@ typedef struct WindowStatePerAggData
 	bool		restart;		/* need to restart this agg in this cycle? */
 } WindowStatePerAggData;
 
+/*
+ * Map between Var attno in a target list and the parsed attno.
+ */
+typedef struct AttnoMap {
+	List		*attno;			/* att number in target list (list of AttNumber) */
+	List		*attnosyn;		/* parsed att number (list of AttNumber) */
+} AttnoMap;
+
 static void initialize_windowaggregate(WindowAggState *winstate,
 									   WindowStatePerFunc perfuncstate,
 									   WindowStatePerAgg peraggstate);
@@ -195,9 +204,9 @@ static Datum GetAggInitVal(Datum textInitVal, Oid transtype);
 
 static bool are_peers(WindowAggState *winstate, TupleTableSlot *slot1,
 					  TupleTableSlot *slot2);
-static bool window_gettupleslot(WindowObject winobj, int64 pos,
-								TupleTableSlot *slot);
 
+static void attno_map(Node *node, AttnoMap *map);
+static bool attno_map_walker(Node *node, void *context);
 
 /*
  * initialize_windowaggregate
@@ -2388,6 +2397,12 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags)
 	TupleDesc	scanDesc;
 	ListCell   *l;
 
+	TargetEntry	*te;
+	Expr		*expr;
+	Var			*var;
+	int			nargs;
+	AttnoMap	attnomap;
+
 	/* check for unsupported flags */
 	Assert(!(eflags & (EXEC_FLAG_BACKWARD | EXEC_FLAG_MARK)));
 
@@ -2483,6 +2498,16 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags)
 	winstate->temp_slot_2 = ExecInitExtraTupleSlot(estate, scanDesc,
 												   &TTSOpsMinimalTuple);
 
+	winstate->prev_slot = ExecInitExtraTupleSlot(estate, scanDesc,
+												 &TTSOpsMinimalTuple);
+
+	winstate->next_slot = ExecInitExtraTupleSlot(estate, scanDesc,
+												 &TTSOpsMinimalTuple);
+
+	winstate->null_slot = ExecInitExtraTupleSlot(estate, scanDesc,
+												 &TTSOpsMinimalTuple);
+	winstate->null_slot = ExecStoreAllNullTuple(winstate->null_slot);
+
 	/*
 	 * create frame head and tail slots only if needed (must create slots in
 	 * exactly the same cases that update_frameheadpos and update_frametailpos
@@ -2667,6 +2692,71 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags)
 	winstate->inRangeAsc = node->inRangeAsc;
 	winstate->inRangeNullsFirst = node->inRangeNullsFirst;
 
+	/* Set up SKIP TO type */
+	winstate->rpSkipTo = node->rpSkipTo;
+	/* Set up row pattern recognition PATTERN clause */
+	winstate->patternVariableList = node->patternVariable;
+	winstate->patternRegexpList = node->patternRegexp;
+
+	/* Set up row pattern recognition DEFINE clause */
+
+	/*
+	 * Collect mapping between varattno and varattnosyn in the targetlist.
+	 * XXX: For now we only check RPR's argument. Eventually we have to
+	 * recurse the targetlist to find out all mappings in Var nodes.
+	 */
+	attnomap.attno = NIL;
+	attnomap.attnosyn = NIL;
+
+	foreach (l, node->plan.targetlist)
+	{
+		te = lfirst(l);
+		if (IsA(te->expr, WindowFunc))
+		{
+			WindowFunc	*func = (WindowFunc *)te->expr;
+			if (func->winfnoid != F_RPR)
+				continue;
+
+			/* sanity check */
+			nargs = list_length(func->args);
+			if (list_length(func->args) != 1)
+				elog(ERROR, "RPR must have 1 argument but function %d has %d args", func->winfnoid, nargs);
+
+			expr = (Expr *) lfirst(list_head(func->args));
+			if (!IsA(expr, Var))
+				elog(ERROR, "RPR's arg is not Var");
+
+			var = (Var *)expr;
+			elog(DEBUG1, "resname: %s varattno: %d varattnosyn: %d",
+				 te->resname, var->varattno, var->varattnosyn);
+			attnomap.attno = lappend_int(attnomap.attno, var->varattno);
+			attnomap.attnosyn = lappend_int(attnomap.attnosyn, var->varattnosyn);
+		}
+	}
+
+	winstate->defineVariableList = NIL;
+	winstate->defineClauseList = NIL;
+	if (node->defineClause != NIL)
+	{
+		foreach(l, node->defineClause)
+		{
+			char		*name;
+			ExprState	*exps;
+
+			te = lfirst(l);
+			name = te->resname;
+			expr = te->expr;
+
+			elog(DEBUG1, "defineVariable name: %s", name);
+			winstate->defineVariableList = lappend(winstate->defineVariableList,
+												   makeString(pstrdup(name)));
+			/* tweak expr so that it referes to outer slot */
+			attno_map((Node *)expr, &attnomap);
+			exps = ExecInitExpr(expr, (PlanState *) winstate);
+			winstate->defineClauseList = lappend(winstate->defineClauseList, exps);
+		}
+	}
+
 	winstate->all_first = true;
 	winstate->partition_spooled = false;
 	winstate->more_partitions = false;
@@ -2674,6 +2764,76 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags)
 	return winstate;
 }
 
+/*
+ * Rewrite Var node's varattno to the varattno which is used in the target
+ * list using AttnoMap.  We also rewrite varno so that it sees outer tuple
+ * (PREV) or inner tuple (NEXT).
+ */
+static void
+attno_map(Node *node, AttnoMap *map)
+{
+	(void) expression_tree_walker(node, attno_map_walker, (void *) map);
+}
+
+static bool
+attno_map_walker(Node *node, void *context)
+{
+	FuncExpr	*func;
+	int			nargs;
+	Expr		*expr;
+	Var			*var;
+	AttnoMap	*attnomap;
+	ListCell	*lc1, *lc2;
+	
+	if (node == NULL)
+		return false;
+
+	attnomap = (AttnoMap *) context;
+
+	if (IsA(node, FuncExpr))
+	{
+		func = (FuncExpr *)node;
+
+		if (func->funcid == F_PREV || func->funcid == F_NEXT)
+		{
+			/* sanity check */
+			nargs = list_length(func->args);
+			if (list_length(func->args) != 1)
+				elog(ERROR, "PREV/NEXT must have 1 argument but function %d has %d args", func->funcid, nargs);
+
+			expr = (Expr *) lfirst(list_head(func->args));
+			if (!IsA(expr, Var))
+				elog(ERROR, "PREV/NEXT's arg is not Var");	/* XXX: is it possible that arg type is Const? */
+			var = (Var *)expr;
+
+			if (func->funcid == F_PREV)
+				var->varno = OUTER_VAR;
+			else
+				var->varno = INNER_VAR;
+		}
+		return expression_tree_walker(node, attno_map_walker, (void *) context);
+	}
+	else if (IsA(node, Var))
+	{
+		var = (Var *)node;	 
+
+		elog(DEBUG1, "original varno: %d varattno: %d", var->varno, var->varattno);
+
+		forboth(lc1, attnomap->attno, lc2, attnomap->attnosyn)
+		{
+			int	attno = lfirst_int(lc1);
+			int	attnosyn = lfirst_int(lc2);
+
+			if (var->varattno == attnosyn)
+			{
+				elog(DEBUG1, "loc: %d rewrite varattno from: %d to %d", var->location, attnosyn, attno);
+				var->varattno = attno;
+			}
+		}
+	}
+	return expression_tree_walker(node, attno_map_walker, (void *) context);
+}
+
 /* -----------------
  * ExecEndWindowAgg
  * -----------------
@@ -2691,6 +2851,8 @@ ExecEndWindowAgg(WindowAggState *node)
 	ExecClearTuple(node->agg_row_slot);
 	ExecClearTuple(node->temp_slot_1);
 	ExecClearTuple(node->temp_slot_2);
+	ExecClearTuple(node->prev_slot);
+	ExecClearTuple(node->next_slot);
 	if (node->framehead_slot)
 		ExecClearTuple(node->framehead_slot);
 	if (node->frametail_slot)
@@ -2740,6 +2902,8 @@ ExecReScanWindowAgg(WindowAggState *node)
 	ExecClearTuple(node->agg_row_slot);
 	ExecClearTuple(node->temp_slot_1);
 	ExecClearTuple(node->temp_slot_2);
+	ExecClearTuple(node->prev_slot);
+	ExecClearTuple(node->next_slot);
 	if (node->framehead_slot)
 		ExecClearTuple(node->framehead_slot);
 	if (node->frametail_slot)
@@ -3080,7 +3244,7 @@ are_peers(WindowAggState *winstate, TupleTableSlot *slot1,
  *
  * Returns true if successful, false if no such row
  */
-static bool
+bool
 window_gettupleslot(WindowObject winobj, int64 pos, TupleTableSlot *slot)
 {
 	WindowAggState *winstate = winobj->winstate;
@@ -3420,14 +3584,53 @@ WinGetFuncArgInFrame(WindowObject winobj, int argno,
 	WindowAggState *winstate;
 	ExprContext *econtext;
 	TupleTableSlot *slot;
-	int64		abs_pos;
-	int64		mark_pos;
 
 	Assert(WindowObjectIsValid(winobj));
 	winstate = winobj->winstate;
 	econtext = winstate->ss.ps.ps_ExprContext;
 	slot = winstate->temp_slot_1;
 
+	if (WinGetSlotInFrame(winobj, slot,
+						  relpos, seektype, set_mark,
+						  isnull, isout) == 0)
+	{
+		econtext->ecxt_outertuple = slot;
+		return ExecEvalExpr((ExprState *) list_nth(winobj->argstates, argno),
+							econtext, isnull);
+	}
+
+	if (isout)
+		*isout = true;
+	*isnull = true;
+	return (Datum) 0;
+}
+
+/*
+ * WinGetSlotInFrame
+ * slot: TupleTableSlot to store the result
+ * relpos: signed rowcount offset from the seek position
+ * seektype: WINDOW_SEEK_HEAD or WINDOW_SEEK_TAIL
+ * set_mark: If the row is found/in frame and set_mark is true, the mark is
+ *		moved to the row as a side-effect.
+ * isnull: output argument, receives isnull status of result
+ * isout: output argument, set to indicate whether target row position
+ *		is out of frame (can pass NULL if caller doesn't care about this)
+ *
+ * Returns 0 if we successfullt got the slot. false if out of frame.
+ * (also isout is set)
+ */
+int
+WinGetSlotInFrame(WindowObject winobj, TupleTableSlot *slot,
+					 int relpos, int seektype, bool set_mark,
+					 bool *isnull, bool *isout)
+{
+	WindowAggState *winstate;
+	int64		abs_pos;
+	int64		mark_pos;
+
+	Assert(WindowObjectIsValid(winobj));
+	winstate = winobj->winstate;
+
 	switch (seektype)
 	{
 		case WINDOW_SEEK_CURRENT:
@@ -3583,15 +3786,13 @@ WinGetFuncArgInFrame(WindowObject winobj, int argno,
 		*isout = false;
 	if (set_mark)
 		WinSetMarkPosition(winobj, mark_pos);
-	econtext->ecxt_outertuple = slot;
-	return ExecEvalExpr((ExprState *) list_nth(winobj->argstates, argno),
-						econtext, isnull);
+	return 0;
 
 out_of_frame:
 	if (isout)
 		*isout = true;
 	*isnull = true;
-	return (Datum) 0;
+	return -1;
 }
 
 /*
@@ -3622,3 +3823,9 @@ WinGetFuncArgCurrent(WindowObject winobj, int argno, bool *isnull)
 	return ExecEvalExpr((ExprState *) list_nth(winobj->argstates, argno),
 						econtext, isnull);
 }
+
+WindowAggState *
+WinGetAggState(WindowObject winobj)
+{
+	return winobj->winstate;
+}
diff --git a/src/backend/utils/adt/windowfuncs.c b/src/backend/utils/adt/windowfuncs.c
index b87a624fb2..74ef11ce55 100644
--- a/src/backend/utils/adt/windowfuncs.c
+++ b/src/backend/utils/adt/windowfuncs.c
@@ -13,6 +13,9 @@
  */
 #include "postgres.h"
 
+#include "catalog/pg_collation_d.h"
+#include "executor/executor.h"
+#include "nodes/execnodes.h"
 #include "nodes/supportnodes.h"
 #include "utils/builtins.h"
 #include "windowapi.h"
@@ -36,10 +39,21 @@ typedef struct
 	int64		remainder;		/* (total rows) % (bucket num) */
 } ntile_context;
 
+/*
+ * rpr process information.
+ * Used for AFTER MATCH SKIP PAST LAST ROW
+ */
+typedef struct SkipContext
+{
+	int64		pos;	/* last row absolute position */
+} SkipContext;
+
 static bool rank_up(WindowObject winobj);
 static Datum leadlag_common(FunctionCallInfo fcinfo,
 							bool forward, bool withoffset, bool withdefault);
-
+static bool get_slots(WindowObject winobj, WindowAggState *winstate, int current_pos);
+static int evaluate_pattern(WindowObject winobj, WindowAggState *winstate,
+							int relpos, char *vname, char *quantifier, bool *result);
 
 /*
  * utility routine for *_rank functions.
@@ -713,3 +727,289 @@ window_nth_value(PG_FUNCTION_ARGS)
 
 	PG_RETURN_DATUM(result);
 }
+
+/*
+ * rpr
+ * allow to use "Row pattern recognition: WINDOW clause" (SQL:2016 R020) in
+ * the target list.
+ * Usage: SELECT rpr(colname) OVER (..)
+ * where colname is defined in PATTERN clause.
+ */
+Datum
+window_rpr(PG_FUNCTION_ARGS)
+{
+#define	MAX_PATTERNS	16	/* max variables in PATTERN clause */
+
+	WindowObject winobj = PG_WINDOW_OBJECT();
+	WindowAggState	*winstate = WinGetAggState(winobj);
+	Datum		result;
+	bool		expression_result;
+	bool		isnull;
+	int			relpos;
+	int64		curr_pos, markpos;
+	ListCell	*lc, *lc1;
+	SkipContext	*context = NULL;
+
+	curr_pos = WinGetCurrentPosition(winobj);
+	elog(DEBUG1, "rpr is called. row: " INT64_FORMAT, curr_pos);
+
+	if (winstate->rpSkipTo == ST_PAST_LAST_ROW)
+	{
+		context = (SkipContext *) WinGetPartitionLocalMemory(winobj, sizeof(SkipContext));
+		if (curr_pos < context->pos)
+		{
+			elog(DEBUG1, "skip this row: curr_pos: " INT64_FORMAT "context->pos: " INT64_FORMAT,
+				 curr_pos, context->pos);
+			PG_RETURN_NULL();
+		}
+	}
+
+	/*
+	 * Evaluate PATTERN until one of expressions is not true or out of frame.
+	 */
+	relpos = 0;
+
+	forboth(lc, winstate->patternVariableList, lc1, winstate->patternRegexpList)
+	{
+		char	*vname = strVal(lfirst(lc));
+		char	*quantifier = strVal(lfirst(lc1));
+
+		elog(DEBUG1, "relpos: %d pattern vname: %s quantifier: %s", relpos, vname, quantifier);
+
+		/* evaluate row pattern against current row */
+		relpos = evaluate_pattern(winobj, winstate, relpos, vname, quantifier, &expression_result);
+
+		/*
+		 * If the expression did not match, we are done.
+		 */
+		if (!expression_result)
+			break;
+
+		/* out of frame? */
+		if (relpos < 0)
+			break;
+
+		/* count up relative row position */
+		relpos++;
+	}
+
+	elog(DEBUG1, "relpos: %d", relpos);
+
+	/*
+	 * If current row satified the pattern, return argument expression.
+	 */
+	if (expression_result)
+	{
+		result = WinGetFuncArgInFrame(winobj, 0,
+									  0, WINDOW_SEEK_HEAD, false,
+									  &isnull, NULL);
+	}
+
+	/*
+	 * At this point we can set mark down to current pos -2.
+	 */
+	markpos = curr_pos -2;
+	elog(DEBUG1, "markpos: " INT64_FORMAT, markpos);
+	if (markpos >= 0)
+		WinSetMarkPosition(winobj, markpos);
+
+	if (expression_result)
+	{
+		if (winstate->rpSkipTo == ST_PAST_LAST_ROW)
+		{
+			context->pos += relpos;
+			elog(DEBUG1, "context->pos: " INT64_FORMAT, context->pos);
+		}
+		PG_RETURN_DATUM(result);
+	}
+
+	PG_RETURN_NULL();
+}
+
+/*
+ * Evaluate expression associated with PATTERN variable vname.
+ * relpos is relative row position in a frame (starting from 0).
+ * "quantifier" is the quatifier part of the PATTERN regular expression.
+ * Currently only '+' is allowed.
+ * result is out paramater representing the expression evaluation result
+ * is true of false.
+ * Return values are:
+ * >=0: the last match relative row position
+ * -1: current row is out of frame
+ */
+static
+int evaluate_pattern(WindowObject winobj, WindowAggState *winstate,
+					 int relpos, char *vname, char *quantifier, bool *result)
+{
+	ExprContext	*econtext = winstate->ss.ps.ps_ExprContext;
+	ListCell	*lc1, *lc2;
+	ExprState	*pat;
+	Datum		eval_result;
+	int			sts;
+	bool		out_of_frame = false;
+	bool		isnull;
+	StringInfo	encoded_str = makeStringInfo();
+	char		pattern_str[128];
+
+	forboth (lc1, winstate->defineVariableList, lc2, winstate->defineClauseList)
+	{
+		char	*name = strVal(lfirst(lc1));
+		bool	second_try_match = false;
+
+		if (strcmp(vname, name))
+			continue;
+
+		/* set expression to evaluate */
+		pat = lfirst(lc2);
+
+		for (;;)
+		{
+			if (!get_slots(winobj, winstate, relpos))
+			{
+				out_of_frame = true;
+				break;	/* current row is out of frame */
+			}
+
+			/* evaluate the expression */
+			eval_result = ExecEvalExpr(pat, econtext, &isnull);
+			if (isnull)
+			{
+				/* expression is NULL */
+				elog(DEBUG1, "expression for %s is NULL at row: %d", vname, relpos);
+				break;
+			}
+			else
+			{
+				if (!DatumGetBool(eval_result))
+				{
+					/* expression is false */
+					elog(DEBUG1, "expression for %s is false at row: %d", vname, relpos);
+					break;
+				}
+				else
+				{
+					/* expression is true */
+					elog(DEBUG1, "expression for %s is true at row: %d", vname, relpos);
+					appendStringInfoChar(encoded_str, vname[0]);
+
+					/* If quantifier is "+", we need to look for more matching row */
+					if (quantifier && !strcmp(quantifier, "+"))
+					{
+						/* remember that we want to try another row */
+						second_try_match = true;
+						relpos++;
+					}
+					else
+						break;
+				}
+			}
+		}
+		if (second_try_match)
+			relpos--;
+
+		if (out_of_frame)
+		{
+			*result = false;
+			return -1;
+		}
+
+		/* build regular expression */
+		snprintf(pattern_str, sizeof(pattern_str), "%c%s", vname[0], quantifier);
+
+		/*
+		 * Do regular expression matching against sequence of rows satisfying
+		 * the expression using regexp_instr().
+		 */
+		sts = DatumGetInt32(DirectFunctionCall2Coll(regexp_instr, DEFAULT_COLLATION_OID,
+													PointerGetDatum(cstring_to_text(encoded_str->data)),
+													PointerGetDatum(cstring_to_text(pattern_str))));
+		elog(DEBUG1, "regexp_instr returned: %d. str: %s regexp: %s",
+			 sts, encoded_str->data, pattern_str);
+		*result = (sts > 0)? true : false;
+	}
+	return relpos;
+}
+
+/*
+ * Get current, previous and next tuple.
+ * Returns true if still within frame.
+ */
+static bool
+get_slots(WindowObject winobj, WindowAggState *winstate, int current_pos)
+{
+	TupleTableSlot *slot;
+	bool	isnull, isout;
+	int		sts;
+	ExprContext *econtext;
+
+	econtext = winstate->ss.ps.ps_ExprContext;
+
+	/* for current row */
+	slot = winstate->temp_slot_1;
+	sts = WinGetSlotInFrame(winobj, slot,
+							current_pos, WINDOW_SEEK_HEAD, false,
+							&isnull, &isout);
+	if (sts < 0)
+	{
+		elog(DEBUG1, "current row is out of frame");
+		econtext->ecxt_scantuple = winstate->null_slot;
+		return false;
+	}
+	else
+		econtext->ecxt_scantuple = slot;
+
+	/* for PREV */
+	if (current_pos > 0)
+	{
+		slot = winstate->prev_slot;
+		sts = WinGetSlotInFrame(winobj, slot,
+								current_pos - 1, WINDOW_SEEK_HEAD, false,
+								&isnull, &isout);
+		if (sts < 0)
+		{
+			elog(DEBUG1, "previous row out of frame at: %d", current_pos);
+			econtext->ecxt_outertuple = winstate->null_slot;
+		}
+		econtext->ecxt_outertuple = slot;
+	}
+	else
+		econtext->ecxt_outertuple = winstate->null_slot;
+
+	/* for NEXT */
+	slot = winstate->next_slot;
+	sts = WinGetSlotInFrame(winobj, slot,
+							current_pos + 1, WINDOW_SEEK_HEAD, false,
+							&isnull, &isout);
+	if (sts < 0)
+	{
+		elog(DEBUG1, "next row out of frame at: %d", current_pos);
+		econtext->ecxt_innertuple = winstate->null_slot;
+	}
+	else
+		econtext->ecxt_innertuple = slot;
+
+	return true;
+}
+
+/*
+ * prev
+ * Dummy function to invoke RPR's navigation operator "PREV".
+ * This is *not* a window function.
+ */
+Datum
+window_prev(PG_FUNCTION_ARGS)
+{
+	PG_RETURN_DATUM(PG_GETARG_DATUM(0));
+}
+
+/*
+ * next
+ * Dummy function to invoke RPR's navigation operation "NEXT".
+ * This is *not* a window function.
+ */
+Datum
+window_next(PG_FUNCTION_ARGS)
+{
+	PG_RETURN_DATUM(PG_GETARG_DATUM(0));
+}
+
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 6996073989..e3a9e0ffeb 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -10397,6 +10397,15 @@
 { oid => '3114', descr => 'fetch the Nth row value',
   proname => 'nth_value', prokind => 'w', prorettype => 'anyelement',
   proargtypes => 'anyelement int4', prosrc => 'window_nth_value' },
+{ oid => '6122', descr => 'row pattern recognition in window',
+  proname => 'rpr', prokind => 'w', prorettype => 'anyelement',
+  proargtypes => 'anyelement', prosrc => 'window_rpr' },
+{ oid => '6123', descr => 'previous value',
+  proname => 'prev', provolatile => 's', prorettype => 'anyelement',
+  proargtypes => 'anyelement', prosrc => 'window_prev' },
+{ oid => '6124', descr => 'next value',
+  proname => 'next', provolatile => 's', prorettype => 'anyelement',
+  proargtypes => 'anyelement', prosrc => 'window_next' },
 
 # functions for range types
 { oid => '3832', descr => 'I/O',
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index cb714f4a19..1643eaa6f1 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -2519,6 +2519,14 @@ typedef struct WindowAggState
 	int64		groupheadpos;	/* current row's peer group head position */
 	int64		grouptailpos;	/* " " " " tail position (group end+1) */
 
+	/* these fields are used in Row pattern recognition: */
+	RPSkipTo	rpSkipTo;		/* Row Pattern Skip To type */	
+	List	   *patternVariableList;	/* list of row pattern variables names (list of String) */
+	List	   *patternRegexpList;	/* list of row pattern regular expressions ('+' or ''. list of String) */
+	List	   *defineVariableList;	/* list of row pattern definition variables (list of String) */
+	List	   *defineClauseList;	/* expression for row pattern definition
+									 * search conditions ExprState list */
+
 	MemoryContext partcontext;	/* context for partition-lifespan data */
 	MemoryContext aggcontext;	/* shared context for aggregate working data */
 	MemoryContext curaggcontext;	/* current aggregate's working data */
@@ -2555,6 +2563,11 @@ typedef struct WindowAggState
 	TupleTableSlot *agg_row_slot;
 	TupleTableSlot *temp_slot_1;
 	TupleTableSlot *temp_slot_2;
+
+	/* temporary slots for RPR */
+	TupleTableSlot *prev_slot;	/* PREV row navigation operator */
+	TupleTableSlot *next_slot;	/* NEXT row navigation operator */
+	TupleTableSlot *null_slot;	/* all NULL slot */
 } WindowAggState;
 
 /* ----------------
diff --git a/src/include/windowapi.h b/src/include/windowapi.h
index b8c2c565d1..a0facf38fe 100644
--- a/src/include/windowapi.h
+++ b/src/include/windowapi.h
@@ -58,7 +58,16 @@ extern Datum WinGetFuncArgInFrame(WindowObject winobj, int argno,
 								  int relpos, int seektype, bool set_mark,
 								  bool *isnull, bool *isout);
 
+extern int WinGetSlotInFrame(WindowObject winobj, TupleTableSlot *slot,
+							 int relpos, int seektype, bool set_mark,
+							 bool *isnull, bool *isout);
+
 extern Datum WinGetFuncArgCurrent(WindowObject winobj, int argno,
 								  bool *isnull);
 
+extern WindowAggState *WinGetAggState(WindowObject winobj);
+
+extern bool window_gettupleslot(WindowObject winobj, int64 pos, TupleTableSlot *slot);
+
+
 #endif							/* WINDOWAPI_H */
-- 
2.25.1


----Next_Part(Mon_Jun_26_17_45_07_2023_724)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v2-0005-Row-pattern-recognition-patch-docs.patch"



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

* Re: explain analyze rows=%.0f
@ 2023-07-03 16:34  Daniel Gustafsson <[email protected]>
  parent: Ibrar Ahmed <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: Daniel Gustafsson @ 2023-07-03 16:34 UTC (permalink / raw)
  To: Ibrar Ahmed <[email protected]>; +Cc: Gregory Stark (as CFM) <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]

> On 8 Jun 2023, at 19:49, Ibrar Ahmed <[email protected]> wrote:
> On Mon, Mar 20, 2023 at 7:56 PM Gregory Stark (as CFM) <[email protected] <mailto:[email protected]>> wrote:

> This patch was marked Returned with Feedback and then later Waiting on
> Author. And it hasn't had any updates since January. What is the state
> on this feedback? If it's already done we can set the patch to Ready
> for Commit and if not do you disagree with the proposed changes?
> 
> If there is a consensus to modify the test cases' output, I am willing to
> make the necessary changes and adjust the patch accordingly. However,
> if there is a preference to keep the output of certain test cases unchanged,
> I can rebase and modify the patch accordingly to accommodate those preferences.

As there hasn't been any other comments I suggest updating your patch to
address Tom's comments to see if we can make progress here.

--
Daniel Gustafsson







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

* Re: explain analyze rows=%.0f
@ 2023-08-01 20:29  Daniel Gustafsson <[email protected]>
  parent: Daniel Gustafsson <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: Daniel Gustafsson @ 2023-08-01 20:29 UTC (permalink / raw)
  To: Ibrar Ahmed <[email protected]>; +Cc: Gregory Stark (as CFM) <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]

> On 3 Jul 2023, at 18:34, Daniel Gustafsson <[email protected]> wrote:
> 
>> On 8 Jun 2023, at 19:49, Ibrar Ahmed <[email protected]> wrote:
>> On Mon, Mar 20, 2023 at 7:56 PM Gregory Stark (as CFM) <[email protected] <mailto:[email protected]>> wrote:
> 
>> This patch was marked Returned with Feedback and then later Waiting on
>> Author. And it hasn't had any updates since January. What is the state
>> on this feedback? If it's already done we can set the patch to Ready
>> for Commit and if not do you disagree with the proposed changes?
>> 
>> If there is a consensus to modify the test cases' output, I am willing to
>> make the necessary changes and adjust the patch accordingly. However,
>> if there is a preference to keep the output of certain test cases unchanged,
>> I can rebase and modify the patch accordingly to accommodate those preferences.
> 
> As there hasn't been any other comments I suggest updating your patch to
> address Tom's comments to see if we can make progress here.

Since there hasn't been any updates here, and the thread has been stalled, I'm
marking this returned with feedback.  Please feel free to resubmit a version of
the patch addressing comments to a future CF.

--
Daniel Gustafsson







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

* Re: explain analyze rows=%.0f
@ 2024-12-12 11:56  Ilia Evdokimov <[email protected]>
  parent: Daniel Gustafsson <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: Ilia Evdokimov @ 2024-12-12 11:56 UTC (permalink / raw)
  To: Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; +Cc: Gregory Stark (as CFM) <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]


On 01.08.2023 23:29, Daniel Gustafsson wrote:
>> On 3 Jul 2023, at 18:34, Daniel Gustafsson <[email protected]> wrote:
>>
>>> On 8 Jun 2023, at 19:49, Ibrar Ahmed <[email protected]> wrote:
>>> On Mon, Mar 20, 2023 at 7:56 PM Gregory Stark (as CFM) <[email protected] <mailto:[email protected]>> wrote:
>>> This patch was marked Returned with Feedback and then later Waiting on
>>> Author. And it hasn't had any updates since January. What is the state
>>> on this feedback? If it's already done we can set the patch to Ready
>>> for Commit and if not do you disagree with the proposed changes?
>>>
>>> If there is a consensus to modify the test cases' output, I am willing to
>>> make the necessary changes and adjust the patch accordingly. However,
>>> if there is a preference to keep the output of certain test cases unchanged,
>>> I can rebase and modify the patch accordingly to accommodate those preferences.
>> As there hasn't been any other comments I suggest updating your patch to
>> address Tom's comments to see if we can make progress here.
> Since there hasn't been any updates here, and the thread has been stalled, I'm
> marking this returned with feedback.  Please feel free to resubmit a version of
> the patch addressing comments to a future CF.
>
> --
> Daniel Gustafsson
>
>
>

Hi everybody,

When the total number of returned tuples is less than the number of 
loops currently shows 'rows = 0'. This can mislead users into thinking 
that no rows were returned at all, even though some might have appeared 
occasionally. For example, if there is 1 tuple over 100 loops, the 
average is 0.01 rows per loop, but as displayed it simply looks like zero.

To clarify this situation, it has been suggested that display rows with 
two decimal places in scenarios where 'loops > 1 && ntuples < loops'. In 
other words, show something like 'rows = 0.01' instead of 'rows=0'. This 
minor change would make it evident that rows did occur, just very 
infrequently.

For all other cases, the current formatting would remain the same. This 
approach aims to provide a more nuanced understanding of query execution 
behavior without introducing unnecessary complexity or false precision.

I would appreciate any thoughts or feedback on this proposal.

--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.


Attachments:

  [text/x-patch] explain_float_row_v4.patch (4.4K, ../../[email protected]/2-explain_float_row_v4.patch)
  download | inline diff:
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index a201ed3082..b3f7fa8743 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -1981,14 +1981,14 @@ ExplainNode(PlanState *planstate, List *ancestors,
 
 		if (es->format == EXPLAIN_FORMAT_TEXT)
 		{
+			appendStringInfo(es->str, " (actual");
 			if (es->timing)
-				appendStringInfo(es->str,
-								 " (actual time=%.3f..%.3f rows=%.0f loops=%.0f)",
-								 startup_ms, total_ms, rows, nloops);
+				appendStringInfo(es->str, " time=%.3f..%.3f", startup_ms, total_ms);
+
+			if (nloops > 1 && planstate->instrument->ntuples < nloops)
+				appendStringInfo(es->str," rows=%.2f loops=%.2f)", rows, nloops);
 			else
-				appendStringInfo(es->str,
-								 " (actual rows=%.0f loops=%.0f)",
-								 rows, nloops);
+				appendStringInfo(es->str," rows=%.0f loops=%.0f)", rows, nloops);
 		}
 		else
 		{
@@ -1999,8 +1999,16 @@ ExplainNode(PlanState *planstate, List *ancestors,
 				ExplainPropertyFloat("Actual Total Time", "ms", total_ms,
 									 3, es);
 			}
-			ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
-			ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+			if (nloops > 1 && planstate->instrument->ntuples < nloops)
+			{
+				ExplainPropertyFloat("Actual Rows", NULL, rows, 2, es);
+				ExplainPropertyFloat("Actual Loops", NULL, nloops, 2, es);
+			}
+			else
+			{
+				ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
+				ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+			}
 		}
 	}
 	else if (es->analyze)
@@ -2052,14 +2060,14 @@ ExplainNode(PlanState *planstate, List *ancestors,
 			if (es->format == EXPLAIN_FORMAT_TEXT)
 			{
 				ExplainIndentText(es);
+				appendStringInfo(es->str, "actual");
 				if (es->timing)
-					appendStringInfo(es->str,
-									 "actual time=%.3f..%.3f rows=%.0f loops=%.0f\n",
-									 startup_ms, total_ms, rows, nloops);
+					appendStringInfo(es->str, " time=%.3f..%.3f", startup_ms, total_ms);
+
+				if (nloops > 1 && planstate->instrument->ntuples < nloops)
+					appendStringInfo(es->str," rows=%.2f loops=%.2f)", rows, nloops);
 				else
-					appendStringInfo(es->str,
-									 "actual rows=%.0f loops=%.0f\n",
-									 rows, nloops);
+					appendStringInfo(es->str," rows=%.0f loops=%.0f)", rows, nloops);
 			}
 			else
 			{
@@ -2070,8 +2078,17 @@ ExplainNode(PlanState *planstate, List *ancestors,
 					ExplainPropertyFloat("Actual Total Time", "ms",
 										 total_ms, 3, es);
 				}
-				ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
-				ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+
+				if (nloops > 1 && planstate->instrument->ntuples < nloops)
+				{
+					ExplainPropertyFloat("Actual Rows", NULL, rows, 2, es);
+					ExplainPropertyFloat("Actual Loops", NULL, nloops, 2, es);
+				}
+				else
+				{
+					ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
+					ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+				}
 			}
 
 			ExplainCloseWorker(n, es);
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index c52bc40e81..cf50776ef7 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -3041,16 +3041,16 @@ select * from tbl1 inner join tprt on tbl1.col1 > tprt.col1;
 
 explain (analyze, costs off, summary off, timing off, buffers off)
 select * from tbl1 inner join tprt on tbl1.col1 = tprt.col1;
-                                QUERY PLAN                                
---------------------------------------------------------------------------
+                                   QUERY PLAN                                   
+--------------------------------------------------------------------------------
  Nested Loop (actual rows=3 loops=1)
    ->  Seq Scan on tbl1 (actual rows=5 loops=1)
-   ->  Append (actual rows=1 loops=5)
+   ->  Append (actual rows=0.60 loops=5.00)
          ->  Index Scan using tprt1_idx on tprt_1 (never executed)
                Index Cond: (col1 = tbl1.col1)
          ->  Index Scan using tprt2_idx on tprt_2 (actual rows=1 loops=2)
                Index Cond: (col1 = tbl1.col1)
-         ->  Index Scan using tprt3_idx on tprt_3 (actual rows=0 loops=3)
+         ->  Index Scan using tprt3_idx on tprt_3 (actual rows=0.33 loops=3.00)
                Index Cond: (col1 = tbl1.col1)
          ->  Index Scan using tprt4_idx on tprt_4 (never executed)
                Index Cond: (col1 = tbl1.col1)


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

* Re: explain analyze rows=%.0f
@ 2025-01-11 09:15  Guillaume Lelarge <[email protected]>
  parent: Ilia Evdokimov <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: Guillaume Lelarge @ 2025-01-11 09:15 UTC (permalink / raw)
  To: Ilia Evdokimov <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]

Hello,

Le jeu. 12 déc. 2024 à 12:57, Ilia Evdokimov <[email protected]>
a écrit :

>
> On 01.08.2023 23:29, Daniel Gustafsson wrote:
> >> On 3 Jul 2023, at 18:34, Daniel Gustafsson <[email protected]> wrote:
> >>
> >>> On 8 Jun 2023, at 19:49, Ibrar Ahmed <[email protected]> wrote:
> >>> On Mon, Mar 20, 2023 at 7:56 PM Gregory Stark (as CFM) <
> [email protected] <mailto:[email protected]>> wrote:
> >>> This patch was marked Returned with Feedback and then later Waiting on
> >>> Author. And it hasn't had any updates since January. What is the state
> >>> on this feedback? If it's already done we can set the patch to Ready
> >>> for Commit and if not do you disagree with the proposed changes?
> >>>
> >>> If there is a consensus to modify the test cases' output, I am willing
> to
> >>> make the necessary changes and adjust the patch accordingly. However,
> >>> if there is a preference to keep the output of certain test cases
> unchanged,
> >>> I can rebase and modify the patch accordingly to accommodate those
> preferences.
> >> As there hasn't been any other comments I suggest updating your patch to
> >> address Tom's comments to see if we can make progress here.
> > Since there hasn't been any updates here, and the thread has been
> stalled, I'm
> > marking this returned with feedback.  Please feel free to resubmit a
> version of
> > the patch addressing comments to a future CF.
> >
> > --
> > Daniel Gustafsson
> >
> >
> >
>
> Hi everybody,
>
> When the total number of returned tuples is less than the number of
> loops currently shows 'rows = 0'. This can mislead users into thinking
> that no rows were returned at all, even though some might have appeared
> occasionally. For example, if there is 1 tuple over 100 loops, the
> average is 0.01 rows per loop, but as displayed it simply looks like zero.
>
> To clarify this situation, it has been suggested that display rows with
> two decimal places in scenarios where 'loops > 1 && ntuples < loops'. In
> other words, show something like 'rows = 0.01' instead of 'rows=0'. This
> minor change would make it evident that rows did occur, just very
> infrequently.
>
> For all other cases, the current formatting would remain the same. This
> approach aims to provide a more nuanced understanding of query execution
> behavior without introducing unnecessary complexity or false precision.
>
> I would appreciate any thoughts or feedback on this proposal.
>
>
Thanks for your patch, this looks like a very interesting feature that I'd
like to see in a future release.

It did a quick run: patch OK, make OK, make install OK, but make check
fails quite a lot on partition_prune.sql.

I guess it would need some work on partition_prune.sql tests and perhaps
also on the docs.

Thanks again.


-- 
Guillaume.


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

* Re: explain analyze rows=%.0f
@ 2025-01-11 11:10  Ilia Evdokimov <[email protected]>
  parent: Guillaume Lelarge <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: Ilia Evdokimov @ 2025-01-11 11:10 UTC (permalink / raw)
  To: Guillaume Lelarge <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]


On 11.01.2025 12:15, Guillaume Lelarge wrote:
>
>
> Thanks for your patch, this looks like a very interesting feature that 
> I'd like to see in a future release.
>
> It did a quick run: patch OK, make OK, make install OK, but make check 
> fails quite a lot on partition_prune.sql.
>
> I guess it would need some work on partition_prune.sql tests and 
> perhaps also on the docs.
>
> Thanks again.
>
>
> -- 
> Guillaume.


Yes, certainly. I have fixed partition_prune.sql. In the documentation 
example for EXPLAIN ANALYZE where loops is greater than one, I updated 
how 'rows' and 'loops' values are displayed so they appear as decimal 
fractions with two digits after the decimal point.

I attached fixed patch.

Any suggestions?

--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.


Attachments:

  [text/x-patch] v5-0001-Clarify-display-of-rows-and-loops-as-decimal-fraction.patch (7.1K, ../../[email protected]/3-v5-0001-Clarify-display-of-rows-and-loops-as-decimal-fraction.patch)
  download | inline diff:
From a934fbd278350edf7a3a3e7fe620a0027ceb2580 Mon Sep 17 00:00:00 2001
From: Ilia Evdokimov <[email protected]>
Date: Sat, 11 Jan 2025 13:59:34 +0300
Subject: [PATCH] Clarify display of rows and loops as decimal fractions

When the average number of rows is small compared to the number of loops,
both rows and loops are displayed as decimal fractions with two digits
after the decimal point in EXPLAIN ANALYZE.
---
 doc/src/sgml/perform.sgml                     |  2 +-
 src/backend/commands/explain.c                | 49 +++++++++++++------
 src/test/regress/expected/partition_prune.out | 10 ++--
 src/test/regress/sql/partition_prune.sql      |  2 +-
 4 files changed, 40 insertions(+), 23 deletions(-)

diff --git a/doc/src/sgml/perform.sgml b/doc/src/sgml/perform.sgml
index a502a2aaba..da7a19f4b8 100644
--- a/doc/src/sgml/perform.sgml
+++ b/doc/src/sgml/perform.sgml
@@ -730,7 +730,7 @@ WHERE t1.unique1 &lt; 10 AND t1.unique2 = t2.unique2;
          -&gt;  Bitmap Index Scan on tenk1_unique1  (cost=0.00..4.36 rows=10 width=0) (actual time=0.004..0.004 rows=10 loops=1)
                Index Cond: (unique1 &lt; 10)
                Buffers: shared hit=2
-   -&gt;  Index Scan using tenk2_unique2 on tenk2 t2  (cost=0.29..7.90 rows=1 width=244) (actual time=0.003..0.003 rows=1 loops=10)
+   -&gt;  Index Scan using tenk2_unique2 on tenk2 t2  (cost=0.29..7.90 rows=1 width=244) (actual time=0.003..0.003 rows=1.00 loops=10.00)
          Index Cond: (unique2 = t1.unique2)
          Buffers: shared hit=24 read=6
  Planning:
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index c24e66f82e..200294b756 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -1981,14 +1981,14 @@ ExplainNode(PlanState *planstate, List *ancestors,
 
 		if (es->format == EXPLAIN_FORMAT_TEXT)
 		{
+			appendStringInfo(es->str, " (actual");
 			if (es->timing)
-				appendStringInfo(es->str,
-								 " (actual time=%.3f..%.3f rows=%.0f loops=%.0f)",
-								 startup_ms, total_ms, rows, nloops);
+				appendStringInfo(es->str, " time=%.3f..%.3f", startup_ms, total_ms);
+
+			if (nloops > 1 && planstate->instrument->ntuples < nloops)
+				appendStringInfo(es->str," rows=%.2f loops=%.2f)", rows, nloops);
 			else
-				appendStringInfo(es->str,
-								 " (actual rows=%.0f loops=%.0f)",
-								 rows, nloops);
+				appendStringInfo(es->str," rows=%.0f loops=%.0f)", rows, nloops);
 		}
 		else
 		{
@@ -1999,8 +1999,16 @@ ExplainNode(PlanState *planstate, List *ancestors,
 				ExplainPropertyFloat("Actual Total Time", "ms", total_ms,
 									 3, es);
 			}
-			ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
-			ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+			if (nloops > 1 && planstate->instrument->ntuples < nloops)
+			{
+				ExplainPropertyFloat("Actual Rows", NULL, rows, 2, es);
+				ExplainPropertyFloat("Actual Loops", NULL, nloops, 2, es);
+			}
+			else
+			{
+				ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
+				ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+			}
 		}
 	}
 	else if (es->analyze)
@@ -2052,14 +2060,14 @@ ExplainNode(PlanState *planstate, List *ancestors,
 			if (es->format == EXPLAIN_FORMAT_TEXT)
 			{
 				ExplainIndentText(es);
+				appendStringInfo(es->str, "actual");
 				if (es->timing)
-					appendStringInfo(es->str,
-									 "actual time=%.3f..%.3f rows=%.0f loops=%.0f\n",
-									 startup_ms, total_ms, rows, nloops);
+					appendStringInfo(es->str, " time=%.3f..%.3f", startup_ms, total_ms);
+
+				if (nloops > 1 && planstate->instrument->ntuples < nloops)
+					appendStringInfo(es->str," rows=%.2f loops=%.2f)", rows, nloops);
 				else
-					appendStringInfo(es->str,
-									 "actual rows=%.0f loops=%.0f\n",
-									 rows, nloops);
+					appendStringInfo(es->str," rows=%.0f loops=%.0f)", rows, nloops);
 			}
 			else
 			{
@@ -2070,8 +2078,17 @@ ExplainNode(PlanState *planstate, List *ancestors,
 					ExplainPropertyFloat("Actual Total Time", "ms",
 										 total_ms, 3, es);
 				}
-				ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
-				ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+
+				if (nloops > 1 && planstate->instrument->ntuples < nloops)
+				{
+					ExplainPropertyFloat("Actual Rows", NULL, rows, 2, es);
+					ExplainPropertyFloat("Actual Loops", NULL, nloops, 2, es);
+				}
+				else
+				{
+					ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
+					ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
+				}
 			}
 
 			ExplainCloseWorker(n, es);
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index c52bc40e81..dc6a3cb54a 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -2338,7 +2338,7 @@ begin
             $1)
     loop
         ln := regexp_replace(ln, 'Workers Launched: \d+', 'Workers Launched: N');
-        ln := regexp_replace(ln, 'actual rows=\d+ loops=\d+', 'actual rows=N loops=N');
+        ln := regexp_replace(ln, 'actual rows=\d+(?:\.\d+)? loops=\d+(?:\.\d+)?', 'actual rows=N loops=N');
         ln := regexp_replace(ln, 'Rows Removed by Filter: \d+', 'Rows Removed by Filter: N');
         return next ln;
     end loop;
@@ -3041,16 +3041,16 @@ select * from tbl1 inner join tprt on tbl1.col1 > tprt.col1;
 
 explain (analyze, costs off, summary off, timing off, buffers off)
 select * from tbl1 inner join tprt on tbl1.col1 = tprt.col1;
-                                QUERY PLAN                                
---------------------------------------------------------------------------
+                                   QUERY PLAN                                   
+--------------------------------------------------------------------------------
  Nested Loop (actual rows=3 loops=1)
    ->  Seq Scan on tbl1 (actual rows=5 loops=1)
-   ->  Append (actual rows=1 loops=5)
+   ->  Append (actual rows=0.60 loops=5.00)
          ->  Index Scan using tprt1_idx on tprt_1 (never executed)
                Index Cond: (col1 = tbl1.col1)
          ->  Index Scan using tprt2_idx on tprt_2 (actual rows=1 loops=2)
                Index Cond: (col1 = tbl1.col1)
-         ->  Index Scan using tprt3_idx on tprt_3 (actual rows=0 loops=3)
+         ->  Index Scan using tprt3_idx on tprt_3 (actual rows=0.33 loops=3.00)
                Index Cond: (col1 = tbl1.col1)
          ->  Index Scan using tprt4_idx on tprt_4 (never executed)
                Index Cond: (col1 = tbl1.col1)
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
index d67598d5c7..bb1e09a536 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -571,7 +571,7 @@ begin
             $1)
     loop
         ln := regexp_replace(ln, 'Workers Launched: \d+', 'Workers Launched: N');
-        ln := regexp_replace(ln, 'actual rows=\d+ loops=\d+', 'actual rows=N loops=N');
+        ln := regexp_replace(ln, 'actual rows=\d+(?:\.\d+)? loops=\d+(?:\.\d+)?', 'actual rows=N loops=N');
         ln := regexp_replace(ln, 'Rows Removed by Filter: \d+', 'Rows Removed by Filter: N');
         return next ln;
     end loop;
-- 
2.34.1



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

* Re: explain analyze rows=%.0f
@ 2025-01-11 14:02  Guillaume Lelarge <[email protected]>
  parent: Ilia Evdokimov <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: Guillaume Lelarge @ 2025-01-11 14:02 UTC (permalink / raw)
  To: Ilia Evdokimov <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]

Le sam. 11 janv. 2025 à 12:10, Ilia Evdokimov <[email protected]>
a écrit :

>
> On 11.01.2025 12:15, Guillaume Lelarge wrote:
>
>
>
> Thanks for your patch, this looks like a very interesting feature that I'd
> like to see in a future release.
>
> It did a quick run: patch OK, make OK, make install OK, but make check
> fails quite a lot on partition_prune.sql.
>
> I guess it would need some work on partition_prune.sql tests and perhaps
> also on the docs.
>
> Thanks again.
>
>
> --
> Guillaume.
>
>
> Yes, certainly. I have fixed partition_prune.sql. In the documentation
> example for EXPLAIN ANALYZE where loops is greater than one, I updated how
> 'rows' and 'loops' values are displayed so they appear as decimal fractions
> with two digits after the decimal point.
>
> I attached fixed patch.
>
> Any suggestions?
>

Thanks for the updated patch.

"make check" is happy, so no more suggestions, except you should register
it in the next commitfest.


-- 
Guillaume.


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

* Re: explain analyze rows=%.0f
@ 2025-01-11 18:21  Ilia Evdokimov <[email protected]>
  parent: Guillaume Lelarge <[email protected]>
  0 siblings, 0 replies; 9+ messages in thread

From: Ilia Evdokimov @ 2025-01-11 18:21 UTC (permalink / raw)
  To: Guillaume Lelarge <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; Ibrar Ahmed <[email protected]>; Gregory Stark (as CFM) <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; vignesh C <[email protected]>; David G. Johnston <[email protected]>; [email protected]


On 11.01.2025 17:02, Guillaume Lelarge wrote:
> Le sam. 11 janv. 2025 à 12:10, Ilia Evdokimov 
> <[email protected]> a écrit :
>
>
>     On 11.01.2025 12:15, Guillaume Lelarge wrote:
>>
>>
>>     Thanks for your patch, this looks like a very interesting feature
>>     that I'd like to see in a future release.
>>
>>     It did a quick run: patch OK, make OK, make install OK, but make
>>     check fails quite a lot on partition_prune.sql.
>>
>>     I guess it would need some work on partition_prune.sql tests and
>>     perhaps also on the docs.
>>
>>     Thanks again.
>>
>>
>>     -- 
>>     Guillaume.
>
>
>     Yes, certainly. I have fixed partition_prune.sql. In the
>     documentation example for EXPLAIN ANALYZE where loops is greater
>     than one, I updated how 'rows' and 'loops' values are displayed so
>     they appear as decimal fractions with two digits after the decimal
>     point.
>
>     I attached fixed patch.
>
>     Any suggestions?
>
>
> Thanks for the updated patch.
>
> "make check" is happy, so no more suggestions, except you should 
> register it in the next commitfest.
>
>
> -- 
> Guillaume.


Done!

https://commitfest.postgresql.org/52/5501/

--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.


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


end of thread, other threads:[~2025-01-11 18:21 UTC | newest]

Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-06-08 17:49 Re: explain analyze rows=%.0f Ibrar Ahmed <[email protected]>
2023-07-03 16:34 ` Daniel Gustafsson <[email protected]>
2023-08-01 20:29   ` Daniel Gustafsson <[email protected]>
2024-12-12 11:56     ` Ilia Evdokimov <[email protected]>
2025-01-11 09:15       ` Guillaume Lelarge <[email protected]>
2025-01-11 11:10         ` Ilia Evdokimov <[email protected]>
2025-01-11 14:02           ` Guillaume Lelarge <[email protected]>
2025-01-11 18:21             ` Ilia Evdokimov <[email protected]>
2023-06-26 08:05 [PATCH v2 4/7] Row pattern recognition patch (executor). Tatsuo Ishii <[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