MIME-Version: 1.0
References: 
 <CACJufxH_Z5aaYEir3=GHoZu-0pKzQdScu85LgCm1C8n=oQo=4Q@mail.gmail.com>
 <CAAAe_zAe8CRN-ggNhcP7b-ALiRrHxKTXBWbNVMhdW_QoO=1c4Q@mail.gmail.com>
 <CACJufxHEu+zYWdpi4gP7FbL+fjMhk8VOF7LJDU4CG=ZMYEPb-w@mail.gmail.com>
 <20260620.121719.209332541992434500.ishii@postgresql.org>
In-Reply-To: <20260620.121719.209332541992434500.ishii@postgresql.org>
Reply-To: assam258@gmail.com
From: Henson Choi <assam258@gmail.com>
Date: Sun, 21 Jun 2026 15:52:20 +0900
Message-ID: 
 <CAAAe_zCKh1ucJVJ0hsoiA-jCoveP2VC7-cpzTZ3D=B-HPhv0uA@mail.gmail.com>
Subject: Re: Row pattern recognition
To: jian.universality@gmail.com
Cc: Tatsuo Ishii <ishii@postgresql.org>, 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="0000000000005277130654bdf86a"
Archived-At: 
 <https://www.postgresql.org/message-id/CAAAe_zCKh1ucJVJ0hsoiA-jCoveP2VC7-cpzTZ3D%3DB-HPhv0uA%40mail.gmail.com>
Precedence: bulk

--0000000000005277130654bdf86a
Content-Type: text/plain; charset="UTF-8"

> CREATE TABLE stock (company TEXT, tdate DATE, price INTEGER);
> CREATE TEMP TABLE stock (company TEXT, tdate DATE, price INTEGER);
> SELECT count(*) over w FROM stock WINDOW w AS ( ROWS BETWEEN CURRENT
> ROW AND UNBOUNDED FOLLOWING PATTERN (A) DEFINE A AS
> pg_temp.stock.price > 0 );
> SELECT count(*) over w FROM stock WINDOW w AS ( ROWS BETWEEN CURRENT
> ROW AND UNBOUNDED FOLLOWING PATTERN (A) DEFINE A AS public.stock.price
>> 0 );
> SELECT count(*) over w FROM stock WINDOW w AS ( ROWS BETWEEN CURRENT
> ROW AND UNBOUNDED FOLLOWING PATTERN (A) DEFINE A AS stock.price > 0 );
>
> The error messages for the above 3 SELECT queries are different.
> (pg_temp.stock.price, public.stock.price, stock.price) mean the same
> thing: column reference,
> Should we try to make the error messages consistent?

Tatsuo has already covered this one, and I'm with him: I'd keep the three
as they are.  They come from three different paths -- a schema-qualified
reference rejected inside DEFINE, a public.stock that simply isn't the
FROM-clause relation, and a range-variable qualification -- so a single
message can't describe all three accurately.

It's worth saying why beyond "different paths".  In the standard a row
pattern variable is itself a range variable -- ISO/IEC 19075-5 4.10 has them
"used to qualify column references" in MEASURES and DEFINE -- so a qualified
name
such as A.price denotes the price column at the rows mapped to pattern
variable A.  That notation collides directly with table/range-variable
qualification: A.price and stock.price are spelled identically, and a
one-component qualifier is ambiguous between a pattern variable and a
relation.  Sorting out that ambiguity is precisely what separates the three
paths above, which is the other reason I'd keep them distinct rather than
fold them into one message now.

We don't implement pattern-variable-qualified references yet -- today an
unqualified column is implicitly the universal row pattern variable, and
that is all DEFINE and MEASURES accept.  I'd like to design and build that
qualifier support once this commit lands; since it sits right on top of the
name resolution you're raising here, I'd genuinely welcome your help with
both the design and the implementation, if you're interested.

> "Range variable qualified expression" is non-standard that may confuse
users.
> To improve clarity and consistency, let's align this with the
> established error pattern:
>
> ERROR: invalid reference to FROM-clause entry for table "the_table"

"invalid reference to FROM-clause entry for table" only fits the middle
case (public.stock); routing (1) and (3) through it would misdescribe them.
On the wording, "range variable qualified" is not really non-standard -- it
is the standard's own framing, the same one above: a row pattern variable
is a range variable, and DEFINE qualification is exactly where that
distinction becomes load-bearing once qualifier resolution lands.  A generic
FROM-clause message would erase the very difference we will need to surface
then, so I'd keep the dedicated wording rather than fold it in.

> What do you all think about renaming validateRPRPatternVarCount to
> preprocessRPRPattern?

Agreed the name is too narrow -- besides checking the count against
RPR_VARID_MAX, the function collects the unique PATTERN variable names into
p_rpr_pattern_vars (the list transformColumnRef uses to spot a
pattern-variable qualifier, and for now to reject A.price).
 preprocessRPRPattern
feels too broad, though.  Rather than pin a name down now, I'd leave some
room: the function's role will shift as the qualifier work lands, so I'll
work out a fitting name as I write that patch.

> The attached v48-0001 mainly replaces PG_INT32_MAX with
> RPR_QUANTITY_INF, but it also includes other changes.
> See the commit message for detail.

Behavior-neutral and a clear improvement; I'll take it.  I'll go through the
details closely and fold in any fixups as I write the patch.

> The attached v48-0002 patch is more about miscellaneous refactoring.
> [...]
> Refactor buildRPRPattern to accept a WindowClause pointer directly.
> [...]
> collectDefineVariables is not needed [...] Also removed
tryUnwrapSingleChild
> [...]
> Slightly adjust variable limits error message.

Also good.  The helper inlining and the WindowClause signature both read as
clear wins; I'll sort out the variable-limit message wording and the rest of
the small details as I fold it.

Thanks again for the careful work.

Best regards,
Henson

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

<div dir=3D"ltr">&gt; CREATE TABLE stock (company TEXT, tdate DATE, price I=
NTEGER);<br>&gt; CREATE TEMP TABLE stock (company TEXT, tdate DATE, price I=
NTEGER);<br>&gt; SELECT count(*) over w FROM stock WINDOW w AS ( ROWS BETWE=
EN CURRENT<br>&gt; ROW AND UNBOUNDED FOLLOWING PATTERN (A) DEFINE A AS<br>&=
gt; pg_temp.stock.price &gt; 0 );<br>&gt; SELECT count(*) over w FROM stock=
 WINDOW w AS ( ROWS BETWEEN CURRENT<br>&gt; ROW AND UNBOUNDED FOLLOWING PAT=
TERN (A) DEFINE A AS public.stock.price<br>&gt;&gt; 0 );<br>&gt; SELECT cou=
nt(*) over w FROM stock WINDOW w AS ( ROWS BETWEEN CURRENT<br>&gt; ROW AND =
UNBOUNDED FOLLOWING PATTERN (A) DEFINE A AS stock.price &gt; 0 );<br>&gt;<b=
r>&gt; The error messages for the above 3 SELECT queries are different.<br>=
&gt; (pg_temp.stock.price, public.stock.price, stock.price) mean the same<b=
r>&gt; thing: column reference,<br>&gt; Should we try to make the error mes=
sages consistent?<br><br>Tatsuo has already covered this one, and I&#39;m w=
ith him: I&#39;d keep the three<br>as they are.=C2=A0 They come from three =
different paths -- a schema-qualified<br>reference rejected inside DEFINE, =
a public.stock that simply isn&#39;t the<br>FROM-clause relation, and a ran=
ge-variable qualification -- so a single<br>message can&#39;t describe all =
three accurately.<br><br>It&#39;s worth saying why beyond &quot;different p=
aths&quot;.=C2=A0 In the standard a row<br>pattern variable is itself a ran=
ge variable -- ISO/IEC 19075-5 4.10 has them<br>&quot;used to qualify colum=
n references&quot; in MEASURES and DEFINE -- so a qualified<br>name<br>such=
 as A.price denotes the price column at the rows mapped to pattern<br>varia=
ble A.=C2=A0 That notation collides directly with table/range-variable<br>q=
ualification: A.price and stock.price are spelled identically, and a<br>one=
-component qualifier is ambiguous between a pattern variable and a<br>relat=
ion.=C2=A0 Sorting out that ambiguity is precisely what separates the three=
<br>paths above, which is the other reason I&#39;d keep them distinct rathe=
r than<br>fold them into one message now.<br><br>We don&#39;t implement pat=
tern-variable-qualified references yet -- today an<br>unqualified column is=
 implicitly the universal row pattern variable, and<br>that is all DEFINE a=
nd MEASURES accept.=C2=A0 I&#39;d like to design and build that<br>qualifie=
r support once this commit lands; since it sits right on top of the<br>name=
 resolution you&#39;re raising here, I&#39;d genuinely welcome your help wi=
th<br>both the design and the implementation, if you&#39;re interested.<br>=
<br>&gt; &quot;Range variable qualified expression&quot; is non-standard th=
at may confuse users.<br>&gt; To improve clarity and consistency, let&#39;s=
 align this with the<br>&gt; established error pattern:<br>&gt;<br>&gt; ERR=
OR: invalid reference to FROM-clause entry for table &quot;the_table&quot;<=
br><br>&quot;invalid reference to FROM-clause entry for table&quot; only fi=
ts the middle<br>case (public.stock); routing (1) and (3) through it would =
misdescribe them.<br>On the wording, &quot;range variable qualified&quot; i=
s not really non-standard -- it<br>is the standard&#39;s own framing, the s=
ame one above: a row pattern variable<br>is a range variable, and DEFINE qu=
alification is exactly where that<br>distinction becomes load-bearing once =
qualifier resolution lands.=C2=A0 A generic<br>FROM-clause message would er=
ase the very difference we will need to surface<br>then, so I&#39;d keep th=
e dedicated wording rather than fold it in.<br><br>&gt; What do you all thi=
nk about renaming validateRPRPatternVarCount to<br>&gt; preprocessRPRPatter=
n?<br><br>Agreed the name is too narrow -- besides checking the count again=
st<br>RPR_VARID_MAX, the function collects the unique PATTERN variable name=
s into<br>p_rpr_pattern_vars (the list transformColumnRef uses to spot a<br=
>pattern-variable qualifier, and for now to reject A.price). =C2=A0preproce=
ssRPRPattern<br>feels too broad, though.=C2=A0 Rather than pin a name down =
now, I&#39;d leave some<br>room: the function&#39;s role will shift as the =
qualifier work lands, so I&#39;ll<br>work out a fitting name as I write tha=
t patch.<br><br>&gt; The attached v48-0001 mainly replaces PG_INT32_MAX wit=
h<br>&gt; RPR_QUANTITY_INF, but it also includes other changes.<br>&gt; See=
 the commit message for detail.<br><br>Behavior-neutral and a clear improve=
ment; I&#39;ll take it.=C2=A0 I&#39;ll go through the<br>details closely an=
d fold in any fixups as I write the patch.<br><br>&gt; The attached v48-000=
2 patch is more about miscellaneous refactoring.<br>&gt; [...]<br>&gt; Refa=
ctor buildRPRPattern to accept a WindowClause pointer directly.<br>&gt; [..=
.]<br>&gt; collectDefineVariables is not needed [...] Also removed tryUnwra=
pSingleChild<br>&gt; [...]<br>&gt; Slightly adjust variable limits error me=
ssage.<br><br>Also good.=C2=A0 The helper inlining and the WindowClause sig=
nature both read as<br>clear wins; I&#39;ll sort out the variable-limit mes=
sage wording and the rest of<br>the small details as I fold it.<br><br>Than=
ks again for the careful work.<br><br>Best regards,<br>Henson<br></div>

--0000000000005277130654bdf86a--