Skip to content

feat: Skip indexes for Azure Cosmos DB for MongoDB #8494

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

Open
wants to merge 2 commits into
base: alpha
Choose a base branch
from

Conversation

EraserKing
Copy link

Pull Request

Issue

Closes: #8492

Approach

By adding a switch in options called "azureCosmosMongoDbCompatibleMode". Setting it to true would bypass some indexes to be created when you would like to use Azure Cosmos DB for MongoDB as the database.

Normally, by default, this is off, and there should not be any impact on existing behaviors.

Tasks

  • Add changes to documentation (guides, repository pages, code comments)

@parse-github-assistant
Copy link

The branch release can only be set as base branch by members of @parse-community/core-maintainers.

Pull requests are usually opened against the default branch alpha, which is the working branch. Different repositories may have base branches with different names. If you are sure you need to open this pull request against a different branch, please ask someone from the team mentioned above.

@parse-github-assistant
Copy link

Thanks for opening this pull request!

@parse-github-assistant parse-github-assistant bot changed the base branch from release to alpha March 31, 2023 02:29
@EraserKing EraserKing changed the title Skip indexes for Azure Cosmos DB for MongoDB feat: Skip indexes for Azure Cosmos DB for MongoDB Mar 31, 2023
@mtrezza
Copy link
Member

mtrezza commented Apr 2, 2023

This should probably be rather a MongoDB adapter option, than an option on the root level of Parse Server.

Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EraserKing EraserKing force-pushed the 8492-disable-index-creation branch from 0d16584 to 700d002 Compare April 3, 2023 02:15
@EraserKing
Copy link
Author

As mentioned in the linked issue, I have made the changes by (a) splitting into separate options (b) move to DatabaseOptions node (seems no MongoDb node? so I think it should be here).
Please review, thanks.

@Moumouls
Copy link
Member

Moumouls commented Apr 8, 2023

@EraserKing @mtrezza maybe related #8042

Comment on lines +557 to +564
:DEFAULT: false */
disableEnsureUsernameCaseInsensitiveIndex: ?boolean;
/* Disables behavior to ensure case-insensitive index on field email on _User collection. Set to `true` if using a database not supporting case-insensitive indexes.
:DEFAULT: false */
disableEnsureEmailCaseInsensitiveIndex: ?boolean;
/* Disables behavior to ensure time to live index on field expire on _Idempotency collection. Set to `true` if using a database not supporting TTL index on this field.
:DEFAULT: false */
disableEnsureIdempotencyExpireIndex: ?boolean;
Copy link
Member

@Moumouls Moumouls Apr 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will introduce security issues to just disable case insensitivity on username and email, i sent a more complete PR to force email and username to lowercase in this cases: #8042

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will introduce security issues to just disable case insensitivity on username and email, i sent a more complete PR to force email and username to lowercase in this cases: #8042

Thanks, I noticed this comment just now
I would update my PR to take your changes once your PR is merged

Copy link
Member

@Moumouls Moumouls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing tests

@mariuszdotnet
Copy link

Is this feature coming anytime soon?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disable case-insensitive index on _User
5 participants