Date: Mon, 01 Jun 2026 11:11:19 +0900 (JST)
Message-Id: <20260601.111119.1029884790276077667.ishii@postgresql.org>
To: assam258@gmail.com
Cc: jian.universality@gmail.com, 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
Subject: Re: Row pattern recognition
From: Tatsuo Ishii <ishii@postgresql.org>
In-Reply-To: 
 <CAAAe_zDDuJofafXyhggPPPLzUXeejH-19NfcLR7jNNQxZchtog@mail.gmail.com>
References: <20260502.140304.670813149418899420.ishii@postgresql.org>
	<20260531.213313.1224322053367141508.ishii@postgresql.org>
	<CAAAe_zDDuJofafXyhggPPPLzUXeejH-19NfcLR7jNNQxZchtog@mail.gmail.com>
Mime-Version: 1.0
Content-Type: Text/Plain; charset=iso-8859-1
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/20260601.111119.1029884790276077667.ishii%40postgresql.org>
Precedence: bulk

Hi Henson,

> Good catch -- you're right, and I'll revert it to static in v48. I
> double-checked the current tree: the only callers left are inside
> parse_clause.c (the findTargetlistEntry wrapper, plus the GROUP BY an=
d ORDER
> BY paths), so the extern prototype in parse_clause.h is now unused. T=
he
> revert is just dropping that prototype and restoring the static quali=
fier
> with the in-file forward declaration it used to have.
> =

> It's worth saying why it went extern in the first place, since the re=
ason is
> no longer visible in the tree:
> =

> The DEFINE clause needs its referenced columns present in the plan's
> targetlist to be evaluable at run time. The original implementation d=
id
> that from the RPR side, in parse_rpr.c, by calling
> findTargetlistEntrySQL99()
> with resjunk =3D true to add the missing entry -- and since that func=
tion was
> static in parse_clause.c, reaching it across files is what required e=
xposing
> it as extern.
> =

> That approach added the whole DEFINE expression to the targetlist, an=
d that
> turned out to be the source of a SIGSEGV: when an RPR window and a pl=
ain
> window coexist, the non-RPR WindowAgg inherited targetlist entries ca=
rrying
> RPRNavExpr nodes it has no way to evaluate. The fix was to add only t=
he Vars
> a DEFINE references (with a guard in allpaths.c to keep those columns=
 from
> being pruned), and that is what removed the cross-file call. So the e=
xtern
> has simply outlived its caller -- exactly as you spotted.
> =

> The one loose end there is an optimization, not a correctness issue: =
the
> allpaths.c guard is deliberately coarse, so it also blocks removing a=

> WindowAgg whose RPR WindowFuncs are all unused. Doing that precisely =
means
> restructuring remove_unused_subquery_outputs(), which runs for every
> subquery and not just RPR -- broad enough that I'm treating it as a
> longer-term item rather than part of this work. It doesn't bring the
> external call back -- the Var-only path stays -- so the revert to sta=
tic is
> safe independently of it.
> =

> I'll fold the static revert into v48.

Thank for eplaining the history.

BTW, in v47-0002 patch,
there are some non ASCII characters ("=A7").

+	/*
+	 * Qualified column references in DEFINE are not supported.  This cov=
ers
+	 * both FROM-clause range variables (prohibited by =A76.5) and patter=
n
+	 * variable qualified names (e.g. UP.price), which are valid per =A74=
.16
+	 * but not yet implemented.
+	 */

Regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp