From: Tom Lane <tgl@sss.pgh.pa.us>
To: Dmitry Dolgov <9erthalion6@gmail.com>
cc: 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>
Subject: Re: pg_stat_statements and "IN" conditions
In-reply-to: <20220310163837.pzvr4yntdk33cbkb@ddolgov.remote.csb>
References: <CALNJ-vRRrdXu9igzh5PFTBghK5hnQWF6gpntTPo_+Agq_TsT5Q@mail.gmail.com> <ec47aa60-0a7b-ea3a-222a-5c6a754c2def@pgmasters.net> <20210318155002.ggrgf2p2mvaodcox@localhost> <20210615151850.6nsue7z5xjhpytle@localhost> <20210616140212.kt3n5wi3alfz5i5d@localhost> <20210930134930.d4vzwhvxhfpk2zxt@localhost> <CALNJ-vSJvaJbcXr46JMyWFT5rO3BdaHO2E=xnAbLNDw1cyUJ-A@mail.gmail.com> <20210930150957.tz3h3unjyrzatw6e@localhost> <4154685.1641337363@sss.pgh.pa.us> <20220105211111.5oh7ypjfonaqubuq@erthalion.local> <20220310163837.pzvr4yntdk33cbkb@ddolgov.remote.csb>
Comments: In-reply-to Dmitry Dolgov <9erthalion6@gmail.com>
	message dated "Thu, 10 Mar 2022 17:38:37 +0100"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <655953.1646932319.1@sss.pgh.pa.us>
Date: Thu, 10 Mar 2022 12:11:59 -0500
Message-ID: <655954.1646932319@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/655954.1646932319%40sss.pgh.pa.us>
Precedence: bulk

Dmitry Dolgov <9erthalion6@gmail.com> writes:
> New status: Waiting on Author

> This seems incorrect, as the only feedback I've got was "this is a bad
> idea", and no reaction on follow-up questions.

I changed the status because it seems to me there is no chance of
this being committed as-is.

1. I think an absolute prerequisite before we could even consider
changing the query jumbler rules this much is to do the work that was
put off when the jumbler was moved into core: that is, provide some
honest support for multiple query-ID generation methods being used at
the same time.  Even if you successfully make a case for
pg_stat_statements to act this way, other consumers of query IDs
aren't going to be happy with it.

2. You haven't made a case for it.  The original complaint was
about different lengths of IN lists not being treated as equivalent,
but this patch has decided to do I'm-not-even-sure-quite-what
about treating different Params as equivalent.  Plus you're trying
to invoke eval_const_expressions in the jumbler; that is absolutely
Not OK, for both safety and semantic reasons.

If you backed off to just treating ArrayExprs containing different
numbers of Consts as equivalent, maybe that'd be something we could
adopt without fixing point 1.  I don't think anything that fuzzes the
treatment of Params can get away with that, though.

			regards, tom lane