> On Thu, Mar 10, 2022 at 12:32:08PM -0500, Robert Haas wrote: > On Thu, Mar 10, 2022 at 12:12 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > > > 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. > > I think there are two separate points here, one about patch quality > and the other about whether the basic idea is good. I think the basic > idea is good. I do not contend that collapsing IN-lists of arbitrary > length is what everyone wants in all cases, but it seems entirely > reasonable to me to think that it is what some people want. So I would > say just make it a parameter and let people configure whichever > behavior they want. My bet is 95% of users would prefer to have it on, > but even if that's wildly wrong, having it as an optional behavior > hurts nobody. Let it be off by default and let those who want it flip > the toggle. On the code quality issue, I haven't read the patch but > your concerns sound well-founded to me from reading what you wrote.
I have the same understanding, there is a toggle in the patch exactly for this purpose. To give a bit more context, the whole development was ORM-driven rather than pulled out of thin air -- people were complaining about huge generated queries that could be barely displayed in monitoring, I was trying to address it via collapsing the list where it was happening. In other words "I'm-not-even-sure-quite-what" part may be indeed too extensive, but was triggered by real world issues. Of course, I could get the implementation not quite right, e.g. I wasn't aware about dangers of using eval_const_expressions. But that's what the CF item and the corresponding discussion is for, I guess. Let me see what I could do to improve it.
