[go: nahoru, domu]

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

[source-mongodb] add subtype when converting binary #42927

Merged
merged 10 commits into from
Aug 29, 2024

Conversation

xiaohansong
Copy link
Contributor
@xiaohansong xiaohansong commented Jul 31, 2024

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?

  • YES 💚
  • NO ❌

@xiaohansong xiaohansong requested a review from a team as a code owner July 31, 2024 23:36
Copy link
vercel bot commented Jul 31, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
airbyte-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 29, 2024 11:00pm

@evantahler
Copy link
Contributor

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());
Copy link
Contributor

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?

Copy link
Contributor Author

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

@octavia-squidington-iii octavia-squidington-iii added the area/documentation Improvements or additions to documentation label Aug 29, 2024
@xiaohansong xiaohansong enabled auto-merge (squash) August 29, 2024 23:05
@xiaohansong xiaohansong merged commit 1b2c27e into master Aug 29, 2024
38 checks passed
@xiaohansong xiaohansong deleted the xiaohan/mongodebug branch August 29, 2024 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/mongodb-v2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants