Thanks for the comments!

> * From what I understand the transformation that's causing this is
>   exactly the one from transformAExprIn, and from that perspective
>   refusing to squash if Vars are present is a reasonable approach (if
>   they won't be splitted off, such Array will not pass
>   IsSquashableConstantList anyway).

Correct. If a list had Vars exclusively, it will not be eligible for squashing.

> * I think one subtle thing, which we're doing wrong in transformAExprIn
>   is splitting elements into rvars and rnonvars, but still using end
>   location of the whole expression a->rexpr_list_end as the end location
>   of the new array. This, together with an attempt to record a constant
>   form the same range is ultimately causing the problem. Probably it
>   would be a good idea to adjust newa->list_end with this in mind.

That crossed my mind, but I am not sure how we can actually do this
since we can have Vars and nonVars in different orders in the list.

```
SELECT * FROM test_squash a, test_squash b WHERE a.id IN (b.id + 1, 1,
b.id + 1, 3,
b.id, b.id + 1);
```

> * Independently from that, it sounds like a good idea to have protection
>   from overlapping constants when generating normalized query. It could
>   be done in the same way as the previous bug was fixed. In
>   fill_in_constant_lengths we currently check for duplicated constants:
>
>     locs[i].location == locs[i - 1].location
>
>   then set length = -1 for those. I think it's worth extending it to
>   check for overlapping with the previous constant, something like:
>
>     (locs[i].location == locs[i - 1].location ||
>      locs[i].location <= locs[i - 1].location + locs[i - 1].length)

Yeah, this may be a good defensive check to add, but it's
not going to be useful for this issue.

--
Sami Imseih
Amazon Web Services (AWS)


Reply via email to