> For example with bind queries like that: > select where $1 in ($3, $2) and 1 in ($4, cast($5 as int)) > \bind 0 1 2 3 4 > > Should we have a bit more coverage, where we use multiple IN and/or > ARRAY lists with constants and/or external parameters?
I will add more test coverage. All the tests we have for constants should also have a external parameter counterpart. > v4-0003 with the extra tests for ARRAY can be applied first, with the > test output slightly adjusted and the casts showing up. That was my mistake in rearranging the v3-0001 as v4-0003. I will fix in the next revision. > Now, looking > independently at v4-0001, it is a bit hard to say what's the direct > benefit of this patch, because nothing in the tests of pgss change > after applying it. Could the benefit of this patch be demonstrated so > as it is possible to compare what's the current vs the what-would-be > new behavior? You're right, this should not be an independent patch. I had intended to eventually merge these v4-0001 and v4-0002 but felt it was cleaner to review separately. I'll just combine them in the next rev. > The patterns generated when using casts is still a bit funny, but > perhaps nobody will bother much about the result generated as these > are uncommon. For example, this gets squashed, with the end of the > cast included: > Q: select where 2 in (1, 4) and 1 in (5, (cast(7 as int)), 6, cast(8 as int)); > R: select where $1 in ($2 /*, ... */) and $3 in ($4 /*, ... */ as int)) > > This does not get squashed: > Q: select where 2 in (1, 4) and > 1 in (5, cast(7 as int), 6, (cast(8 as int)), 9, 10, (cast(8 as text))::int); > R: select where $1 in ($2 /*, ... */) and > $3 in ($4, cast($5 as int), $6, (cast($7 as int)), $8, $9, (cast($10 as > text))::int) > > This is the kind of stuff that should also have coverage for, IMO, or > we will never keep track of what the existing behavior is, and if > things break in some way in the future. This is interesting actually. This is the behavior on HEAD, and I don't get why the first list with the casts does not get squashed, while the second one does. I will check IsSquashableConst tomorrow unless Dmitry gets to it first. ``` test=# select where 2 in (1, 4) and 1 in (5, cast(7 as int), 6, (cast(8 as int)), 9, 10, (cast(8 as text))::int); -- (0 rows) test=# select where 1 in (5, cast(7 as int), 6); -- (0 rows) test=# select queryid, substr(query, 1, 100) as query from pg_stat_statements; queryid | query ----------------------+----------------------------------------------------------------------------------- ------------------- 2125518472894925252 | select where $1 in ($2 /*, ... */) and $3 in ($4, cast($5 as int), $6, (cast($7 as int)), $8, $9, (c -4436613157077978160 | select where $1 in ($2 /*, ... */) ``` > FWIW, with v4-0002 applied, I am seeing one diff in the dml tests, > where a IN list is not squashed for pgss_dml_tab. hmm, I did not observe the same diff. -- Sami