MIME-Version: 1.0
References: <20260224.115625.1966558814200895991.ishii@postgresql.org>
 <CAAAe_zAc4CQDMP5WVuNZq5q6A_CymN5xwrva38df3okrtYdYsw@mail.gmail.com>
 <20260224.140927.1828965853586507533.ishii@postgresql.org> <20260224.204417.914253025244188300.ishii@postgresql.org>
In-Reply-To: <20260224.204417.914253025244188300.ishii@postgresql.org>
Reply-To: assam258@gmail.com
From: Henson Choi <assam258@gmail.com>
Date: Fri, 27 Feb 2026 20:30:57 +0900
Message-ID: <CAAAe_zDaaOH4kaNfeN3Gk9u8VyTpUm-UgFebKc1KKB8o+PSjPg@mail.gmail.com>
Subject: Re: Row pattern recognition
To: Tatsuo Ishii <ishii@postgresql.org>
Cc: vik@postgresfriends.org, er@xs4all.nl, jacob.champion@enterprisedb.com, 
	david.g.johnston@gmail.com, peter@eisentraut.org, 
	pgsql-hackers@postgresql.org
Content-Type: multipart/alternative; boundary="000000000000e297d8064bcc922a"
Archived-At: <https://www.postgresql.org/message-id/CAAAe_zDaaOH4kaNfeN3Gk9u8VyTpUm-UgFebKc1KKB8o%2BPSjPg%40mail.gmail.com>
Precedence: bulk

--000000000000e297d8064bcc922a
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Tatsuo,

Currently we do not account the cost of RPR while planning.  Attached
> is the first attempt to try to estimate the RPR costs. The cost model
> is very simple:
>
> expression cost per PATTERN variable * number of input tuples
>
> Any idea to make this estimation better?
>

 >   foreach(lc, windowFuncs)
>   {
>   ...
> + /* also add DEFINE clause expressions' cost to per-input-row costs */
> + if (winclause->rpPattern)
> + {
> + List   *pattern_vars; /* list of pattern variable names */
> + ListCell   *lc2;
> +
> + pattern_vars =3D collectPatternVariables(winclause->rpPattern);
> +
> + /* iterate according to the pattern variable */
> + foreach(lc2, pattern_vars)
> + {
> + char   *ptname =3D strVal((char *) lfirst(lc2));

`collectPatternVariables` returns a list of String nodes (via
`makeString()`), so `strVal(lfirst(lc2))` is the idiomatic form.
The `(char *)` cast is misleading.

There is also a correctness issue: DEFINE expressions belong to the
window clause, not to individual window functions, so their cost
should not be multiplied by the number of window functions sharing
the clause.

The fix is to compute the DEFINE cost once outside the loop and add
it to `startup_cost` and `total_cost` directly, after the
`foreach(lc, windowFuncs)` block.

Regarding the cost model: the NFA executor evaluates all DEFINE
expressions once per row into a shared `nfaVarMatched[]` array that
all active contexts read from, and contexts advance strictly forward
so no prior row is ever re-evaluated.  The one-evaluation-per-row
cost model is therefore accurate.  NFA-aware cost
modeling could be built on top of this foundation in a separate patch
down the road, once the NFA implementation has matured.

For now, the DEFINE expression costs themselves already serve as a
natural penalty =E2=80=94 a window clause with RPR will consistently appear
more expensive than a comparable plain window function.  This gives
the surrounding plan a reasonable cost signal for decisions such as
join ordering and materialization of RPR subqueries.  So the current
approach is reasonable as a first step.

Other than that, the approach looks good to me.  Would it be okay if
I revise the patch along those lines?

Best regards,
Henson

--000000000000e297d8064bcc922a
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">Hi Tatsuo,<br></div><div dir=3D"ltr"><br>=
</div><div class=3D"gmail_quote gmail_quote_container"><blockquote class=3D=
"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left-width:1px;borde=
r-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">Cur=
rently we do not account the cost of RPR while planning.=C2=A0 Attached<br>
is the first attempt to try to estimate the RPR costs. The cost model<br>
is very simple:<br>
<br>
expression cost per PATTERN variable * number of input tuples<br>
<br>
Any idea to make this estimation better?<br></blockquote><div><br></div><di=
v>=C2=A0&gt; =C2=A0	foreach(lc, windowFuncs)<br></div>&gt; =C2=A0	{<br>&gt;=
 =C2=A0		...<br>&gt; +		/* also add DEFINE clause expressions&#39; cost to =
per-input-row costs */<br>&gt; +		if (winclause-&gt;rpPattern)<br>&gt; +		{=
<br>&gt; +			List	 =C2=A0 *pattern_vars;	/* list of pattern variable names =
*/<br>&gt; +			ListCell =C2=A0 *lc2;<br>&gt; +<br>&gt; +			pattern_vars =3D=
 collectPatternVariables(winclause-&gt;rpPattern);<br>&gt; +<br>&gt; +			/*=
 iterate according to the pattern variable */<br>&gt; +			foreach(lc2, patt=
ern_vars)<br>&gt; +			{<br>&gt; +				char	 =C2=A0 *ptname =3D strVal((char =
*) lfirst(lc2));<br><br>`collectPatternVariables` returns a list of String =
nodes (via<br>`makeString()`), so `strVal(lfirst(lc2))` is the idiomatic fo=
rm.<br>The `(char *)` cast is misleading.<br><br>There is also a correctnes=
s issue: DEFINE expressions belong to the<br>window clause, not to individu=
al window functions, so their cost<br>should not be multiplied by the numbe=
r of window functions sharing<br>the clause.<br><br>The fix is to compute t=
he DEFINE cost once outside the loop and add<br>it to `startup_cost` and `t=
otal_cost` directly, after the<br>`foreach(lc, windowFuncs)` block.<br><br>=
Regarding the cost model: the NFA executor evaluates all DEFINE<br>expressi=
ons once per row into a shared `nfaVarMatched[]` array that<br>all active c=
ontexts read from, and contexts advance strictly forward<br>so no prior row=
 is ever re-evaluated.=C2=A0 The one-evaluation-per-row<br>cost model is th=
erefore accurate.=C2=A0 NFA-aware cost<br>modeling could be built on top of=
 this foundation in a separate patch<br>down the road, once the NFA impleme=
ntation has matured.<br><br>For now, the DEFINE expression costs themselves=
 already serve as a<br>natural penalty =E2=80=94 a window clause with RPR w=
ill consistently appear<br>more expensive than a comparable plain window fu=
nction.=C2=A0 This gives<br>the surrounding plan a reasonable cost signal f=
or decisions such as<br>join ordering and materialization of RPR subqueries=
.=C2=A0 So the current<br>approach is reasonable as a first step.<br><br>Ot=
her than that, the approach looks good to me.=C2=A0 Would it be okay if<br>=
I revise the patch along those lines?<br><br>Best regards,<br>Henson<br></d=
iv></div>

--000000000000e297d8064bcc922a--