Date: Thu, 26 Mar 2020 17:45:41 +0100
From: Julien Rouhaud <rjuju123@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Fujii Masao <masao.fujii@oss.nttdata.com>,
	legrand legrand <legrand_legrand@hotmail.com>,
	pgsql-hackers@postgresql.org
Subject: Re: Patch: to pass query string to pg_plan_query()
Message-ID: <20200326164541.GD80836@nol>
References: <1583789487074-0.post@n3.nabble.com>
 <6ecbaaca-5a8e-5fe6-f7e1-a893e708bd7b@oss.nttdata.com>
 <14854.1585237484@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <14854.1585237484@sss.pgh.pa.us>
Precedence: bulk

On Thu, Mar 26, 2020 at 11:44:44AM -0400, Tom Lane wrote:
> Fujii Masao <masao.fujii@oss.nttdata.com> writes:
> > Does anyone object to this patch? I'm thinking to commit it separetely
> > at first before committing the planning_counter_in_pg_stat_statements
> > patch.
> 
> I took a quick look through v9-0001-Pass-query-string-to-the-planner.patch
> and it's fine by me.  It also matches up with something I've wanted to
> do for awhile, which is to make the query string available during
> planning and execution so that we can produce error cursors for
> run-time errors, when relevant.
> 
> (It's a little weird that the patch doesn't make standard_planner
> actually *do* anything with the string, like say save it into
> the PlannerInfo struct.  But that can come later I guess.)
> 
> Note that I wouldn't want to bet that all of these call sites always have
> non-null query strings to pass; but probably most of the time they will.

Surprinsingly, the whole regression tests pass flawlessly with an non-null
query string assert, but we did had some discussion about it.  The pending IVM
patch would break that assumption, same as some non trivial extensions like
citus (see
https://www.postgresql.org/message-id/flat/CAFMSG9HJQr%3DH8doWJOp%3DwqyKbVqxMLkk_Qu2KfpmkKvS-Xn7qQ%40mail.gmail.com#ab8ea541b8c8464f7b52ba6d8d480b7d
and later), so we didn't make it a hard requirement.