[Minor]: unify ANY/ALL planning and align ANY NULL semantics with PG #21743
Conversation
Jefffrey
left a comment
There was a problem hiding this comment.
Will we need a note in the upgrade guide for these changes?
Previously ANY only supported this case in the table: I'm not sure if I'm equipped to answer this but I don't think it would be necessary. Otherwise, we can mention the case above for the behavior change (to align with PG) |
Jefffrey
left a comment
There was a problem hiding this comment.
Previously ANY only supported
=operator andALLdid not exist for subquery comparisons. My changes to extending these two are not released yet. Thus, only affect is that when an array has unmatched NULLs we'll return NULL instead of false.
Ah ok, if these changes haven't been released yet then we should be good without an upgrade note 👍
berkaysynnada
left a comment
There was a problem hiding this comment.
The unification is a nice simplification, thank you @buraksenn.
One concern worth thinking about unparser roundtrip change. 1 = ANY("array"."left") previously unparsed back to itself; now it expands into a multiple branches of CASE. It may affect the consumers of the unparser, but they might also be capable of optimizing such semantics. I think it's not a blocker for merging.
Which issue does this PR close?
Related with #2547 and #2548 but does not close
Rationale for this change
In #21416 I've aligned ALL operator NULL semantics to Postgres while supporting additional operators. I've implemented ANY operator but missed that part. Initially that PR included these changes but with the suggestion from @Jefffrey I've separated those changes and opened this PR.
What changes are included in this PR?
Refactor ANY operator to use same logic with ALL operator
align null semantics with postgres
5 = ANY(NULL::INT[])NULLNULLfalse5 <> ANY(NULL::INT[])NULLNULLfalse5 > ANY(NULL::INT[])NULLNULLfalse5 < ANY(NULL::INT[])NULLNULLfalse5 >= ANY(NULL::INT[])NULLNULLfalse5 <= ANY(NULL::INT[])NULLNULLfalseI'll explore a followup implementation on ANY and ALL as UDFs instead of this case approach to see if it will perform faster. I've wanted to open this PR to correct out the NULL behavior
Are these changes tested?
Yes existing and additional slt tests.
Are there any user-facing changes?
Yes users will be able to see null semantics are same as postgres