Update Rust SDK and add NDC v0.1.2 support #42
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
dmoverton
reviewed
Apr 17, 2024
| S::BinData => false, | ||
| S::ObjectId => false, | ||
| S::Bool => false, | ||
| S::Null => false, |
There was a problem hiding this comment.
In MongoDB values of any type can be compared and ordered. I get that we may want to be a bit stricter on what we allow, but a few of these I'm not sure about:
- Given
ObjectIdis the type of the primary key in most collections, it might be useful to be orderable. Although I'm not really sure. Boolordering is by convention (and by the rules of Boolean Algebra) usually defined withfalse < true. But maybe that's not useful in a database. What do other connectors do?Nullis trivially orderable as the type has exactly one value (null). But again, I'm not sure if that's practically useful.
@hallettj do you have any thoughts on this?
There was a problem hiding this comment.
@daniel-chambers given that you have just kept the same behaviour as previously, with respect to ordering and comparison, I don't think we need to let any debate on this hold up this PR. Your changes look good so I'll approve it.
There was a problem hiding this comment.
Fair points. I agree with ordering for ObjectId and Bool, sure. I wouldn't bother with null; we'd just be adding complexity to their GraphQL types for no reason.
dmoverton
approved these changes
Apr 17, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 PR updates the Rust SDK to a newer version which brings support for NDC spec v0.1.2. It then updates the connector to support the new Type Representations that were added in v0.1.2.
NDC scalar types are now generated using separate code instead of translating them from v2 scalar types. This is because v2 scalar types don't contain enough information to determine what the type representation should be.
However, the comparison operators and aggregate operators (the most complex bits) are drawn from the same code as v2.
I also did a bit of refactoring and moved some of the properties such as "orderable" and "comparable" onto the BsonScalarType object instance. I implemented it using a flat match expression rather than the existing "look in an array" method, because, while that might make for shorter code, the flat match expression will produce compiler errors if the BSON type ever changes.
JIRA: MDB-88