Skip to content

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

Merged
ebyhr merged 1 commit intotrinodb:masterfrom
sahoss:mongo_id_projection
Jul 4, 2023
Merged

Don't project _id field in Mongo DB connector when it's not required#18081
ebyhr merged 1 commit intotrinodb:masterfrom
sahoss:mongo_id_projection

Conversation

@sahoss
Copy link
Copy Markdown
Contributor

@sahoss sahoss commented Jun 28, 2023

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
@sahoss
Copy link
Copy Markdown
Contributor Author

sahoss commented Jun 29, 2023

@ebyhr addressed comments. please take a look.

@sahoss
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

sahoss commented Jun 29, 2023

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
Copy Markdown
Contributor Author

sahoss commented Jul 3, 2023

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

@ebyhr
Copy link
Copy Markdown
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed mongodb MongoDB connector

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