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

Newest Version Breaks Compound MongoDB Text Indices #4212

Closed
seriph opened this issue Sep 27, 2017 · 10 comments
Closed

Newest Version Breaks Compound MongoDB Text Indices #4212

seriph opened this issue Sep 27, 2017 · 10 comments
Labels
type:question Support or code-level question

Comments

@seriph
Copy link

seriph commented Sep 27, 2017

Issue Description

Just updated our backend to the newest code, and looks like the text index support is faulty when you're using Mongo. Specifically, the code from this commit:
5aafc93

Automatic index creation does not account for compound text indices and will simply iterate through all columns queried for text and try to create an index for each, which isn't how compound indices work on Mongo. I've been running Parse Server with my own text query functionality in production since April, and tried syncing with upstream today but had to comment this out to get it to work.

We worked together on text queries back in June, and the code at that point was fine (the JS API was implemented weirdly, in that it requires a column to query on text even though Mongo itself doesn't, but it worked fine). This update breaks it if you're using compound text indices.

I'm not sure the current approach is workable in the case of compound indices without adding extra per-query overhead to check for them. IMO it would be better if, in the case of a Mongo backend, we allow the user to be completely responsible for text index creation.

Steps to reproduce

Try to query on a text index when using Mongo.

Expected Results

Expect to work.

Actual Outcome

Failing with 'Only one text index is supported, please delete all text indexes to use new field.' introduced by the above commit.

Environment Setup

  • Server

    • parse-server version (Be specific! Don't say 'latest'.) : 2.6.2
    • Operating System: Ubuntu
    • Hardware: An epic server
    • Localhost or remote server? (AWS, Heroku, Azure, Digital Ocean, etc): Heroku
  • Database

    • MongoDB version: 3.4.5 (WiredTiger)
    • Storage engine: WiredTiger
    • Hardware: Too many servers.
    • Localhost or remote server? (AWS, mLab, ObjectRocket, Digital Ocean, etc): mLab

Logs/Trace

@seriph seriph changed the title Newest Version Breaks MongoDB Text Indices Newest Version Breaks Compound MongoDB Text Indices Sep 27, 2017
@montymxb
Copy link
Contributor

montymxb commented Sep 27, 2017

@seriph thanks for bringing this to our attention. This is something that I was pushing for actually. The idea being it is one less thing that would have to be managed with an instance. This would allow greater control over solutions where parse is offered as a service, where direct control may not be possible. However, we were aware that other users would be handling their own index support. The intent here was to not mess with existing indexes, but to allow dynamic creation of a new index if a query is made on a field lacking one.

From what you're saying (and from a quick look back at that commit), we should be doing an additional check to account for compound indexes before we try to generate a new one. It's possible a check put in around here would serve to that end.

@dplewis
Copy link
Member

dplewis commented Sep 28, 2017

@montymxb @flovilmart You mentions index creation on the Schema API. I'm willing to work on it with some godly guidance. If you want to have the discussion here.

New Type Parse Schema?
Add, Remove, Get Indexes?
Will the indexes get stored on parse? Cache / Special Table?
MasterKey Only / Security?

@montymxb
Copy link
Contributor

@dplewis absolutely, it is something that we've needed to do, and we should just get it done. As for the details I'll follow up with what might be a good basis for index management APIs, including functionality, storage, management, security, etc.

@seriph given that this was originally an issue you brought up the primary concern, regardless of where this goes, is to not interfere with existing indexes and hands on management of those indexes. What you are doing should be fine if we proceed to implement an api that allows management of indexes, it would just be an alternative method for convenience.

@seriph
Copy link
Author

seriph commented Sep 28, 2017

This sounds like a good strategy @montymxb

@montymxb
Copy link
Contributor

@dplewis @seriph Okay so here's a basic proposal for how we could implement an Index API. Please note that this is a general synopsis and lacks some details, but it provides overall direction as to where we would need to focus in regards to an implementation.

Per index, counting individuals inside a compound index, we would need the basic CRUD ops. If we utilize the existing schema api at /schemas (which already takes json with a fields parameter specifying which fields to add/delete) we could simply add indexes to it under the following circumstances.

Create
Using POST via /schemas/<className>, creates a new schema entry.
This would be an example of json going upstream.

{
	"indexes" : {
		"my_index"	: {
			...
		}
	}
}

Read
Using GET via /schemas/<className>, or /schemas for all.
Json coming back down, would look pretty much like what we would be sending up for create.

{
	"indexes" : {
		"my_index"	: {
			...
		}
	}
}

Update
Using PUT via /schemas/<className>, updates an existing entry.
I should clarify that although an update op is supported, it would still be just updating an existing schema by adding or removing an index only.
Json would be just about identical to that used during CREATE.

Delete
Using PUT via /schemas/<className>, updates an existing entry, same as above.
Json going upstream, follows what we do for deleting a field via the same api.

{
	"indexes" : {
		"my_index"	: {
			"__op"		: "Delete"
		}
	}
}

There is also the need to consider that if anything needs to be cleaned up when a schema entry is deleted using DELETE via /schemas/<className>, we would do that then.

Most of the sdks should require little modification to allow using this new feature of an existing api.

As it may be hinting above, this suggests that we modify the layout of _Schema to optionally include indexes as a parameter, or _indexes if it might be mistaken for an actual field name; whatever works.

Adding an index would register it under the relevant class entry in _Schema, removing it would take it out.

For working with existing indexes, which would not be registered in our schema, we would have to add an auto-detection during use. If a query using fullText is passed, and the relevant index is NOT found in the schema entry for that class, we should perform a check to find if one has been created through other means already. If we do find one, we should allow the operation and add this index into the relevant schema entry, so as to not incur an additional lookup next time around. Doing it this way, simply running a fullText query would auto-load all the pre-existing indexes (counting those within a compound index) for that class.

Managing compound indexes would be central to this, the schema would simply see that there are one or more indexes, while in reality there may be one index or a compound set of indexes. This is where I am not very familiar with the topic, but I imagine the server side code would have the capacity to automatically manage a simple index or a compound index, and be capable of modifying a singular index to a compound one as needed (or back even).

Let me know what you guys think of this. This is again just a brush over the top and may be missing some critical components, but it should outline what we need to get done in order for this to work.

@dplewis
Copy link
Member

dplewis commented Sep 30, 2017

@montymxb Thanks, this is a great start.

@flovilmart What are your thoughts on @montymxb proposal?

@seriph
Copy link
Author

seriph commented Oct 1, 2017 via email

This was referenced Oct 3, 2017
@flovilmart
Copy link
Contributor

@dplewis should we first revert back, so @seriph 's setup is not completely borked and then see what we can do about those indices?

@dplewis
Copy link
Member

dplewis commented Dec 2, 2017

@seriph Can you update to the latest version of Parse-Server to see if this issue persists?

@dplewis
Copy link
Member

dplewis commented Jan 9, 2018

Addressed via #4240. Please feel free to reopen if issue persists

@dplewis dplewis closed this as completed Jan 9, 2018
@mtrezza mtrezza added type:question Support or code-level question and removed 🔧 troubleshooting labels Jul 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:question Support or code-level question
Projects
None yet
Development

No branches or pull requests

5 participants