Skip to content

fix: operators in, contains, containsany and containsall#1796

Merged
lvca merged 3 commits intomainfrom
issue-1785
Oct 29, 2024
Merged

fix: operators in, contains, containsany and containsall#1796
lvca merged 3 commits intomainfrom
issue-1785

Conversation

@lvca
Copy link
Copy Markdown
Member

@lvca lvca commented Oct 29, 2024

Hi @gramian, this PR fixes #1785.

I've created new test for the missing cases. About your tests, check my comments:

(NULL IN [NULL]) -- is FALSE but should be TRUE <-- OK
([NULL] IN [NULL]) -- is TRUE but should be FALSE <-- WHY?

([NULL] CONTAINS [NULL]) -- is TRUE but should be FALSE <-- WHY?

([NULL] CONTAINSANY NULL) -- is FALSE is fine if the right-hand-side has to be a collection, BUT <-- IT SHOULS RETURN TRUE
([NULL] CONTAINSALL NULL) -- is TRUE which is inconsistent with `CONTAINSANY`<-- OK? SEE ABOVE

@lvca lvca added the bug label Oct 29, 2024
@lvca lvca added this to the 24.11.1 milestone Oct 29, 2024
@lvca lvca requested a review from gramian October 29, 2024 04:35
@lvca lvca self-assigned this Oct 29, 2024
@gramian
Copy link
Copy Markdown
Collaborator

gramian commented Oct 29, 2024

I spelled out the predicates below to clarify my thinking. Please correct me if I am wrong or I am missing something.

  • ([NULL] IN [NULL]) -- is TRUE but should be FALSE <-- WHY?
    • The predicate asks: "Is in the right array an array with one element being a null?" No, as the right array holds only a null. In comparison:
    ([NULL] IN [[NULL]]) -- should be TRUE
    
  • ([NULL] CONTAINS [NULL]) -- is TRUE but should be FALSE <-- WHY?
    • This is the same in reverse: "Does the left array contain an array with one element being null?" No, as the left array holds only a null.
  • [NULL] CONTAINSANY NULL) -- is FALSE is fine if the right-hand-side has to be a collection, BUT <-- IT SHOULS RETURN TRUE
    • I agree by the same logic of above: "Does the left array contain at least one element being a null?" Yes it does.
  • ([NULL] CONTAINSALL NULL) -- is TRUE which is inconsistent with CONTAINSANY<-- OK? SEE ABOVE
    • This is fine.

@lvca
Copy link
Copy Markdown
Member Author

lvca commented Oct 29, 2024

I see it.

In our code, I guess inherited from OrientDB, when both left and right are arrays (collections), it does this:

    if (right instanceof Collection) {
      if (left instanceof Collection<?>)
        return ((Collection<?>) right).containsAll((Collection<?>) left);

I don't know how many use cases are useful when you check if an array is in an array of arrays. Also in classic SQL this seems to not be supported.

We could just document this special behavior: if a collection of elements is tested against another collections of elements IN , then the single values of coll1are checked if contained incoll2`. @gramian WDYT?

@gramian
Copy link
Copy Markdown
Collaborator

gramian commented Oct 29, 2024

@lvca I am fine with that (and can also do that in the docs), but wouldn't that make CONTAINSANY unnecessary?

@lvca
Copy link
Copy Markdown
Member Author

lvca commented Oct 29, 2024

Since you can have a sub query with the IN, I don't see any use case for the containsany...

@gramian
Copy link
Copy Markdown
Collaborator

gramian commented Oct 29, 2024

Besides being more semantic me neither.

@lvca
Copy link
Copy Markdown
Member Author

lvca commented Oct 29, 2024

Fixed the basic cases with null values.

@lvca lvca merged commit a11b782 into main Oct 29, 2024
@robfrank robfrank deleted the issue-1785 branch January 14, 2026 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SQL: Array comparisons with NULL

2 participants