SQL: Implement null safe equality operator <=> #35873
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
This operator handles nulls in different way than the normal `=`. If one of the operants is `null` and the other not it returns `false`. If both operants are `null` it returns `true`. Therefore in contrary to `=`, which returns `null` if at least one of the operants is `null`, this one never returns `null` as a result. Closes: elastic#35871
Collaborator
| Pinging @elastic/es-search |
costin requested changes Nov 24, 2018
Member
costin left a comment
costin left a comment There was a problem hiding this comment.
Looks good. I've added a comment though on the IsNull optimization which should be in the optimizer not the parser as it won't catch non-trivial cases:
ABS(NULL) <=> a
| return new Equals(loc, left, right); | ||
| case SqlBaseParser.NULLEQ: | ||
| // Simplify to IS NULL to avoid special handling in QueryTranslator later on | ||
| if (right.equals(NULL)) { |
Member
There was a problem hiding this comment.
This logic should be optimizer (changing the tree) instead of the parser which should be pretty accurate to the actual query (helps with reporting errors). Further more it would make this apply if one of the expressions becomes NULL in folding and such.
Contributor Author
There was a problem hiding this comment.
You're right, I didn't think this through.
Contributor Author
| @costin thanks for the comment, fixup pushed. |
matriv added a commit that referenced this pull request Nov 26, 2018
This operator handles nulls in different way than the normal `=`. If one of the operants is `null` and the other not it returns `false`. If both operants are `null` it returns `true`. Therefore in contrary to `=`, which returns `null` if at least one of the operants is `null`, this one never returns `null` as a result. Closes: #35871
Contributor Author
| Backported to |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
This operator handles nulls in different way than the normal
=.If one of the operants is
nulland the other not it returnsfalse.If both operants are
nullit returnstrue. Therefore in contrary to=, which returnsnullif at least one of the operants isnull, this onenever returns
nullas a result.Closes: #35871