Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ESQL] Remove unused NullEquals class #108022

Merged
merged 2 commits into from
Apr 30, 2024

Conversation

not-napoleon
Copy link
Member

Although the ES|QL language doesn't support a null equals operator, we had a placeholder class for it with no @Evaluator methods defined. I presume this was to match the QL BinaryComparisonOperator enum, although I wasn't involved in the initial discussions around that choice. This PR removes the unused code, and associated references to it.

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

@@ -227,17 +222,6 @@ public void testDualEqualsConjunction() {
assertEquals(FALSE, exp);
}

// a <=> 1 AND a <=> 2 -> FALSE
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These test cases were just copied over from the QL version of the optimizer rules.

Copy link
Contributor

@alex-spies alex-spies left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @not-napoleon !

@not-napoleon not-napoleon merged commit 62f55be into elastic:main Apr 30, 2024
15 checks passed
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.14 Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 108022

@not-napoleon
Copy link
Member Author

The backport conflicts are related to #105407 (review) which wasn't backported. I don't think this is actually all that important to backport, so I'm just going to leave it as 8.15 only.

Copy link
Contributor

@astefan astefan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

To give a bit of background on what NullEquals is:

  • it was introduced in ES SQL as part of SQL: implement null safe equality operator <=> #35871
  • it returns true / false on equality against null values. Because SQL has a 3 value logic, any equality with null would result in null. This operator helps having a truly null equality

In ESQL we have the IS NULL / IS NOT NULL predicates that do the same thing. ES SQL also has these two predicates, but offered a shortcut in the form of an operator (NullEquals <=>).

ESQL also allows field == null BUT it's requiring a specific casting ie field == null::integer and this equality returns null. In a sense, field == null it's the ESQL variant of ES SQL's field = null.

@costin
Copy link
Member

costin commented Apr 30, 2024

The operator is useful not only when comparing a field against null but also fields against each other - fieldA <=> fieldB and always return true/false even when at least one value is null.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/ES|QL AKA ESQL >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants