Message-ID: <07919318-6c35-a13f-6a1d-98686a3e38de@gmail.com>
Date: Wed, 15 Feb 2023 08:51:56 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: pg_stat_statements and "IN" conditions
Content-Language: en-US
To: Dmitry Dolgov <9erthalion6@gmail.com>
Cc: Sergei Kornilov <sk@zsrv.org>, Michael Paquier <michael@paquier.xyz>,
 Alvaro Herrera <alvherre@alvh.no-ip.org>, Marcos Pegoraro
 <marcos@f10.com.br>, vignesh C <vignesh21@gmail.com>,
 Robert Haas <robertmhaas@gmail.com>, Zhihong Yu <zyu@yugabyte.com>,
 David Steele <david@pgmasters.net>,
 PostgreSQL-development <pgsql-hackers@postgresql.org>,
 Greg Stark <stark@mit.edu>, Pavel Trukhanov <pavel.trukhanov@gmail.com>,
 Tom Lane <tgl@sss.pgh.pa.us>
References: <20230202150554.dwxhvw23epcskppb@ddolgov.remote.csb>
 <20230204170841.xfgxoo7ymwbvu7yx@erthalion.local>
 <Y98GsRJoCM4RrcWp@paquier.xyz>
 <20230205113346.gmbf5ycnukuhnmqa@erthalion.local>
 <2420105.1675612952@sss.pgh.pa.us>
 <20230205195600.e3bhwszzm5kdyyxl@erthalion.local>
 <966631675800892@znolar4klagewshm.sas.yp-c.yandex.net>
 <20230209150238.rdtwruh5i34hdag5@ddolgov.remote.csb>
 <dda65b89-4ebf-261c-de9c-7b91ab9e4047@gmail.com>
 <20230211104707.grsicemegr7d3mgh@erthalion.local>
 <20230211120820.y7krq37jubnjsa5b@erthalion.local>
From: David Geier <geidav.pg@gmail.com>
In-Reply-To: <20230211120820.y7krq37jubnjsa5b@erthalion.local>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: <https://www.postgresql.org/message-id/07919318-6c35-a13f-6a1d-98686a3e38de%40gmail.com>
Precedence: bulk

Hi,

On 2/11/23 13:08, Dmitry Dolgov wrote:
>> On Sat, Feb 11, 2023 at 11:47:07AM +0100, Dmitry Dolgov wrote:
>>
>> The original version of the patch was doing all of this, i.e. handling
>> numerics, Param nodes, RTE_VALUES. The commentary about
>> find_const_walker in tests is referring to a part of that, that was
>> dealing with evaluation of expression to see if it could be reduced to a
>> constant.
>>
>> Unfortunately there was a significant push back from reviewers because
>> of those features. That's why I've reduced the patch to it's minimally
>> useful version, having in mind re-implementing them as follow-up patches
>> in the future. This is the reason as well why I left tests covering all
>> this missing functionality -- as breadcrumbs to already discovered
>> cases, important for the future extensions.
> I'd like to elaborate on this a bit and remind about the origins of the
> patch, as it's lost somewhere in the beginning of the thread. The idea
> is not pulled out of thin air, everything is coming from our attempts to
> improve one particular monitoring infrastructure in a real commercial
> setting. Every covered use case and test in the original proposal was a
> result of field trials, when some application-side library or ORM was
> responsible for gigabytes of data in pgss, chocking the monitoring agent.

Thanks for the clarifications. I didn't mean to contend the usefulness 
of the patch and I wasn't aware that you already jumped through the 
loops of handling Param, etc. Seems like supporting only constants is a 
good starting point. The only thing that is likely confusing for users 
is that NUMERICs (and potentially constants of other types) are 
unsupported. Wouldn't it be fairly simple to support them via something 
like the following?

     is_const(element) || (is_coercion(element) && is_const(element->child))

-- 
David Geier
(ServiceNow)