> 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


Reply via email to