-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
[source-mongodb] add subtype when converting binary #42927
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Good find! |
@@ -128,7 +141,7 @@ private Bson buildFilter() { | |||
case OBJECT_ID -> new BsonObjectId(new ObjectId(state.id())); | |||
case INT -> new BsonInt32(Integer.parseInt(state.id())); | |||
case LONG -> new BsonInt64(Long.parseLong(state.id())); | |||
case BINARY -> parseBinaryIdString(state.id()); | |||
case BINARY -> parseBinaryIdString(state.id(), state.binarySubType()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is intended to ensure there's no multiple binary subtype of binary _id on a single collection.
Am I reading it correctly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not really - this is to make sure we write the correct filter when querying mongo from record iterator
What
#40525
for binary typed
_id
, we first base64 encode them into a readable string, and decode it back to binary and send to mongo as a cursor anchor.When decoding it back to binary, we should also take binary subtype into consideration, otherwise it won't be converted back to the original value.
How
In state message we will also need to track binSubtype. normally it should be null, unless the parent type is Binary. To migrate existing state where it does not have this new type, it will be defaulted to Binary(subtype 0)
Review guide
User Impact
Can this PR be safely reverted and rolled back?