MIME-Version: 1.0
References: 
 <CAAAe_zDDuJofafXyhggPPPLzUXeejH-19NfcLR7jNNQxZchtog@mail.gmail.com>
 <20260601.111119.1029884790276077667.ishii@postgresql.org>
 <CAAAe_zBhzHyazC_Ot+HXBe_nToKc7AHs4r-s0nNcaBPw0L17wA@mail.gmail.com>
 <20260601.114703.1561993497414705173.ishii@postgresql.org>
In-Reply-To: <20260601.114703.1561993497414705173.ishii@postgresql.org>
Reply-To: assam258@gmail.com
From: Henson Choi <assam258@gmail.com>
Date: Mon, 1 Jun 2026 14:35:24 +0900
Message-ID: 
 <CAAAe_zCZc8s4zWfmVVUOt0y_FU=v7YTcJJJ4UL2gBzJ2_KkUmQ@mail.gmail.com>
Subject: Re: Row pattern recognition
To: Tatsuo Ishii <ishii@postgresql.org>, jian.universality@gmail.com
Cc: zsolt.parragi@percona.com, sjjang112233@gmail.com,
 vik@postgresfriends.org,
	er@xs4all.nl, jacob.champion@enterprisedb.com, david.g.johnston@gmail.com,
	peter@eisentraut.org, li.evan.chao@gmail.com, pgsql-hackers@postgresql.org
Content-Type: multipart/alternative; boundary="0000000000007d279606532a90a4"
Archived-At: 
 <https://www.postgresql.org/message-id/CAAAe_zCZc8s4zWfmVVUOt0y_FU%3Dv7YTcJJJ4UL2gBzJ2_KkUmQ%40mail.gmail.com>
Precedence: bulk

--0000000000007d279606532a90a4
Content-Type: text/plain; charset="UTF-8"

Hi Tatsuo, Jian,

While tidying RPR comments I found a small inconsistency in the varId
bounds.
The comment/README side I'm already fixing in the in-progress series;
whether
to also change the bounds is a separate follow-up.  As lead author that one
is
ultimately your call, Tatsuo, but I'd welcome Jian's and the list's input on
it first.

The current state, in src/include/optimizer/rpr.h:

  #define RPR_VARID_MAX   251
  #define RPR_VARID_BEGIN 252   /* control codes 252..255 */
  ... END 253, ALT 254, FIN 255

  RPRElemIsVar(e)  ==  ((e)->varId <= RPR_VARID_MAX)   /* 0..251 */

and the limit enforced in parse_rpr.c:

  if (list_length(*varNames) >= RPR_VARID_MAX)   /* reject the 252nd */
      ereport(ERROR, "too many pattern variables", "Maximum is 251");

So 251 variables are accepted as varId 0..250, leaving 251 a hole: never
assigned, yet the macro still classifies it as a variable -- one wider than
the comment's own "0 to RPR_VARID_MAX - 1".

RPRVarId is a uint8, kept small on purpose: varId is the likely per-row
match-history key, and since a match can run arbitrarily long the history
grows with it -- so one byte per row, not two, is what keeps that footprint
in check.

The catch of staying in uint8: the four control codes already fill 252..255,
so 251 is the only free slot for any future sentinel (anchor ^/$, exclusion
{- -}) short of widening to uint16.  So the hole is really the last reserve.

Three ways, by what the gap is spent on:

(1) Leave it -- just the doc alignment already underway: 251 stays a
documented
    reserve, macro unchanged.  No follow-up commit.  The one free slot is
then
    on hand for a single future control code, should one ever be needed.

(2) Fill it as a 252nd variable (0..251).  Compatible and doable anytime; a
few
    lines in parse_rpr.c / rpr.h plus the boundary test.  But it spends the
    last free slot, so a future control code would then force either a
    compatibility-breaking narrow of RPR_VARID_MAX or a widen to two bytes
    (doubling history).  Maximal variables now, the control question
deferred.

(3) Reserve 16 control codes now (4 used + 12 spare) at the 0xF0 boundary:
    vars 0..239, control 240..255, existing sentinels unmoved, macro becomes
    (varId & 0xF0) != 0xF0.  Buys 12-code headroom inside the byte, so
history
    stays 1 byte and (2)'s fork never arises.  Same edit shape as (2); costs
    only the nominal drop to 240 variables -- but it is a narrowing, so free
    only pre-release.

The asymmetry: (3) is the only one with a deadline -- a narrowing is
compatible
only before release, while (1)/(2) stay open forever.  So the question is
whether to spend this one free moment to lock in 1-byte control headroom
(3),
or stay minimal now (1)/(2) and take the narrow-or-widen later if it is ever
needed.  My own lean is toward (3): 240 variables is already far more than
any
real pattern will use, so the capacity we give up is nominal, while the
12-code
buffer closes the narrow-or-widen fork for good and keeps match history at
one
byte -- and it is the one choice that is free only now.  That said, I'd like
the decision to rest on everyone's input -- Jian's and the list's as much as
mine -- with you, Tatsuo, weighing it all and making the final call.

Either way, once the feature matures and the final control-code count is
known,
the space can be repacked gap-free -- so none of these is the last word.

Which would you prefer?

Thanks,
Henson

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

<div dir=3D"ltr">Hi Tatsuo, Jian,<br><br>While tidying RPR comments I found=
 a small inconsistency in the varId bounds.<br>The comment/README side I=
9;m already fixing in the in-progress series; whether<br>to also change the=
 bounds is a separate follow-up.=C2=A0 As lead author that one is<br>ultima=
tely your call, Tatsuo, but I&#39;d welcome Jian&#39;s and the list&#39;s i=
nput on<br>it first.<br><br>The current state, in src/include/optimizer/rpr=
.h:<br><br>=C2=A0 #define RPR_VARID_MAX =C2=A0 251<br>=C2=A0 #define RPR_VA=
RID_BEGIN 252 =C2=A0 /* control codes 252..255 */<br>=C2=A0 ... END 253, AL=
T 254, FIN 255<br><br>=C2=A0 RPRElemIsVar(e) =C2=A0=3D=3D =C2=A0((e)-&gt;va=
rId &lt;=3D RPR_VARID_MAX) =C2=A0 /* 0..251 */<br><br>and the limit enforce=
d in parse_rpr.c:<br><br>=C2=A0 if (list_length(*varNames) &gt;=3D RPR_VARI=
D_MAX) =C2=A0 /* reject the 252nd */<br>=C2=A0 =C2=A0 =C2=A0 ereport(ERROR,=
 &quot;too many pattern variables&quot;, &quot;Maximum is 251&quot;);<br><b=
r>So 251 variables are accepted as varId 0..250, leaving 251 a hole: never<=
br>assigned, yet the macro still classifies it as a variable -- one wider t=
han<br>the comment&#39;s own &quot;0 to RPR_VARID_MAX - 1&quot;.<br><br>RPR=
VarId is a uint8, kept small on purpose: varId is the likely per-row<br>mat=
ch-history key, and since a match can run arbitrarily long the history<br>g=
rows with it -- so one byte per row, not two, is what keeps that footprint<=
br>in check.<br><br>The catch of staying in uint8: the four control codes a=
lready fill 252..255,<br>so 251 is the only free slot for any future sentin=
el (anchor ^/$, exclusion<br>{- -}) short of widening to uint16.=C2=A0 So t=
he hole is really the last reserve.<br><br>Three ways, by what the gap is s=
pent on:<br><br>(1) Leave it -- just the doc alignment already underway: 25=
1 stays a documented<br>=C2=A0 =C2=A0 reserve, macro unchanged.=C2=A0 No fo=
llow-up commit.=C2=A0 The one free slot is then<br>=C2=A0 =C2=A0 on hand fo=
r a single future control code, should one ever be needed.<br><br>(2) Fill =
it as a 252nd variable (0..251).=C2=A0 Compatible and doable anytime; a few=
<br>=C2=A0 =C2=A0 lines in parse_rpr.c / rpr.h plus the boundary test.=C2=
=A0 But it spends the<br>=C2=A0 =C2=A0 last free slot, so a future control =
code would then force either a<br>=C2=A0 =C2=A0 compatibility-breaking narr=
ow of RPR_VARID_MAX or a widen to two bytes<br>=C2=A0 =C2=A0 (doubling hist=
ory).=C2=A0 Maximal variables now, the control question deferred.<br><br>(3=
) Reserve 16 control codes now (4 used + 12 spare) at the 0xF0 boundary:<br=
>=C2=A0 =C2=A0 vars 0..239, control 240..255, existing sentinels unmoved, m=
acro becomes<br>=C2=A0 =C2=A0 (varId &amp; 0xF0) !=3D 0xF0.=C2=A0 Buys 12-c=
ode headroom inside the byte, so history<br>=C2=A0 =C2=A0 stays 1 byte and =
(2)&#39;s fork never arises.=C2=A0 Same edit shape as (2); costs<br>=C2=A0 =
=C2=A0 only the nominal drop to 240 variables -- but it is a narrowing, so =
free<br>=C2=A0 =C2=A0 only pre-release.<br><br>The asymmetry: (3) is the on=
ly one with a deadline -- a narrowing is compatible<br>only before release,=
 while (1)/(2) stay open forever.=C2=A0 So the question is<br>whether to sp=
end this one free moment to lock in 1-byte control headroom (3),<br>or stay=
 minimal now (1)/(2) and take the narrow-or-widen later if it is ever<br>ne=
eded.=C2=A0 My own lean is toward (3): 240 variables is already far more th=
an any<br>real pattern will use, so the capacity we give up is nominal, whi=
le the 12-code<br>buffer closes the narrow-or-widen fork for good and keeps=
 match history at one<br>byte -- and it is the one choice that is free only=
 now.=C2=A0 That said, I&#39;d like<br>the decision to rest on everyone&#39=
;s input -- Jian&#39;s and the list&#39;s as much as<br>mine -- with you, T=
atsuo, weighing it all and making the final call.<br><br>Either way, once t=
he feature matures and the final control-code count is known,<br>the space =
can be repacked gap-free -- so none of these is the last word.<br><br>Which=
 would you prefer?<br><br>Thanks,<br>Henson<br></div>

--0000000000007d279606532a90a4--