Message-ID: <9171ecda-e1f4-4390-ae54-98bc441b98e3@oss.nttdata.com>
Date: Mon, 9 Jun 2025 19:07:42 +0900
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: BUG #18947: TRAP: failed Assert("len_to_wrt >= 0") in
 pg_stat_statements
Content-Language: en-US
To: Michael Paquier <michael@paquier.xyz>, Dilip Kumar <dilipbalaut@gmail.com>
Cc: a.kozhemyakin@postgrespro.ru, pgsql-bugs@lists.postgresql.org,
 Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com>
References: <18947-cdd2668beffe02bf@postgresql.org>
 <CAFiTN-tGux5MZ9Hk2wKv5sVhk7eDq2AHaT79LxnQbVamxoRn9Q@mail.gmail.com>
 <aET38DIyBht33bIP@paquier.xyz>
From: Fujii Masao <masao.fujii@oss.nttdata.com>
In-Reply-To: <aET38DIyBht33bIP@paquier.xyz>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/9171ecda-e1f4-4390-ae54-98bc441b98e3%40oss.nttdata.com>
Precedence: bulk


On 2025/06/08 11:39, Michael Paquier wrote:
> On Sat, Jun 07, 2025 at 10:19:53PM +0530, Dilip Kumar wrote:
>> I didn't get time to debug, I might look into it tomorrow if someone
>> doesn't do it before that, but I am able to reproduce the issue and
>> seems like problem is with respect to computing the quer_loc in below
>> loop, and due to that 'quer_loc' is becoming bigger than query_len and
>> hitting the assert.
> 
> The loop in charge of assigning the constants in the queries needs to
> be smarter in terms of the new locations assigned to sub-queries, here
> the VALUES one.
> 
> 499edb09741b is the origin of the regression, so open item assigned to
> me (will look into it later), adding Anthonin in CC.

The issue seems to be that the commit missed handling cases where
a clause follows a SELECT query wrapped in parentheses.

If the following clause includes a constant (e.g., LIMIT 1 or
FETCH FIRST 1 ROW ONLY), it can trigger the reported assertion failure.
If it doesn't include a constant (e.g., FETCH FIRST ROW ONLY),
the assertion failure doesn't occur, but only the inner SELECT query (inside
the parentheses) appears in pg_stat_statements, i.e.,
the "FETCH FIRST ROW ONLY" part is missing from pg_stat_statements.query.

+					SelectStmt *n = (SelectStmt *) $2;
+
+					/*
+					 * As SelectStmt's location starts at the SELECT keyword,
+					 * we need to track the length of the SelectStmt within
+					 * parentheses to be able to extract the relevant part
+					 * of the query.  Without this, the RawStmt's length would
+					 * be used and would include the closing parenthesis.
+					 */
+					n->stmt_len = @3 - @2;

Shouldn't this part be skipped in cases where the SELECT with parens is
followed by a clause? At least in those cases, this logic doesn't seem
appropriate.

Regards,

-- 
Fujii Masao
NTT DATA Japan Corporation