Skip to content

Don't project _id field in Mongo DB connector when it's not required #18081

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 4, 2023
Merged

Don't project _id field in Mongo DB connector when it's not required #18081

merged 1 commit into from
Jul 4, 2023

Conversation

sahoss
Copy link
Contributor

@sahoss sahoss commented Jun 28, 2023 •
edited by ebyhr
Loading

Description

Currently, _id field is retrieved even when it's not necessary. We can exclude by https://www.mongodb.com/docs/drivers/java/sync/current/fundamentals/builders/projections/#exclusion-of-_id.

Fixes #17970

Release notes

(x) This is not user-visible or docs only and no release notes are required.

@cla-bot cla-bot bot added the cla-signed label Jun 28, 2023
@sahoss sahoss marked this pull request as ready for review June 28, 2023 21:30
@github-actions github-actions bot added the mongodb MongoDB connector label Jun 28, 2023
ebyhr
ebyhr reviewed Jun 29, 2023
View reviewed changes
@sahoss
Copy link
Contributor Author

sahoss commented Jun 29, 2023

@ebyhr addressed comments. please take a look.

@sahoss
Copy link
Contributor Author

sahoss commented Jun 29, 2023

the tests which are failing are hive/hadoop related. 99% sure not related to this.

@Praveen2112
Copy link
Member

Can we use Builder from MongoDB driver - https://www.mongodb.com/docs/drivers/java/sync/current/fundamentals/builders/#using-builders which has some predefined APIs we could use.

@sahoss
Copy link
Contributor Author

sahoss commented Jun 29, 2023 •
edited
Loading

Can we use Builder from MongoDB driver - https://www.mongodb.com/docs/drivers/java/sync/current/fundamentals/builders/#using-builders which has some predefined APIs we could use.

Would it be better to do it in a different PR? as otherwise this PR would be changing two things 1. excludeId 2. changing implementation of how projections are built(implementation+return type(bson(new) vs document(current)). and neither are currently directly tested. @Praveen2112
so the safer approach imo is to first modify existing code and then refactor to use bson+builder.

@sahoss
Copy link
Contributor Author

sahoss commented Jul 3, 2023

@ebyhr @Praveen2112 can you please take a look? thanks

@ebyhr
Copy link
Member

ebyhr commented Jul 4, 2023

Could you rebase on master to resolve conflicts?

@ebyhr ebyhr merged commit 02f0e0d into trinodb:master Jul 4, 2023
@github-actions github-actions bot added this to the 421 milestone Jul 4, 2023
@sahoss sahoss deleted the mongo_id_projection branch July 5, 2023 07:58
@colebow colebow mentioned this pull request Jul 5, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ebyhr ebyhr ebyhr approved these changes

Assignees
No one assigned
Labels
cla-signed mongodb MongoDB connector
Milestone
421
Development

Successfully merging this pull request may close these issues.

Don't project _id field in Mongo DB connector when it's not required
3 participants
@sahoss @Praveen2112 @ebyhr