On Thu, May 22, 2025 at 03:10:34PM -0500, Sami Imseih wrote: > > IMO adding a struct as suggested is okay, especially if it reduces the > > overall code complexity. But we don't want a node, just a bare struct. > > Adding a node would be more troublesome. > > In v4, a new private struct is added in gram.y, but we are also adding > additional fields to track the expression boundaries to the required > nodes.
Handling external parameters as something that gets squashed is also the consensus I am understanding we've reached. I'm OK with it. Upthread, scenarios with multiple IN lists was mentioned to be broken: https://www.postgresql.org/message-id/CAA5RZ0ts6zb-efiJ+K31Z_YDU=m7the43vv6zbcqqxiabr3...@mail.gmail.com 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? v4-0003 with the extra tests for ARRAY can be applied first, with the test output slightly adjusted and the casts showing up. 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? 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. 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. The squashing tests point to more issues in the v4 series: - Some lists are not getting squashed anymore. - Some spacing issues, like "( $5)::jsonb". Am I missing something? -- Michael
signature.asc
Description: PGP signature
