Full Text Search

[dplewis]

parse-community/parse-server#3904

I've added full text search and sort to ParseQuery.

I didn't add support for $language, $caseSensitive, $diraticSensitive just $term.

To run the tests you need to create a text index in mongo. Because of this fact this PR will fail on travis

mongo localhost:27017/test

db.TestObject.createIndex( { subject: "text" } )

Let me know your thoughts on this.

[montymxb]

@dplewis Quick question. This requires an index to be created for the given key in advance. Is this done server side in any way or am I missing something?

[dplewis]

For the given key in advance. The server side doesn't create the indexes.

[montymxb]

Okay. So if I'm understanding this correctly to utilize the functionality the appropriate text index has be created manually in the database?

[dplewis]

Yes You can only have 1 text index but you can have compound indexes. On the server side we could create a compound text index that includes every string field but that may take up too much ram if I remember correctly.

@flovilmart What are your thoughts?

[flovilmart]

Thoughts on? Index creation or?

[dplewis]

Index creation or just let the users create their own indexes

[flovilmart]

Index creatio. Would likely require schema extensions to support it, I would be willing to get that feature in, on the schema API so we can get it in the dashboard

[montymxb]

Hmmm... My primary concern is that this query parameter doesn't automatically work or set itself up when requested via an sdk. It requires intervention on part by the dev to setup the DB in advance for the query, which is potentially off putting to those who aren't familiar with database management :/.

As a rule of thumb I would say that nothing additional should have to be done to the database on part by the devs to enable additional features within an sdk.

If we're going to do this it should probably automatically generate an index upon an incoming request for fullText (assuming one isn't already present), or some other additional config to the parse-server instance running (like for mail adapters and push configurations).

If indexes (compound or not) are possible via either route, and memory is a direct issue related to this, then those indexes should be able to be removed as well.

[montymxb]

Yeah that could be done via schema. Something like:

// add
$schema->addIndex('key');

// remove
$schema->removeIndex('key');

That would allow reasonable management of the indexes, letting someone add/remove them as needed without having to touch the DB directly 👍 .

$schema being an instance of ParseSchema.

[flovilmart]

I would not allow the remove in the 1st place, it just reclaims memory. But allow for compound index right away

[montymxb]

Additionally if ParseQuery::fullText is dependent on an index being present the server should return an error indicating if one is not present while making a query.

@flovilmart would removing a field by something like ParseSchema::deleteField clean up the index as well then? Just so it's not left floating around, or would mongo/postgres handle that automatically?

[dplewis]

@montymxb Mongo throws an error

Parse\ParseException: {"name":"MongoError","message":"text index required for $text query","ok":0,"errmsg":"text index required for $text query","code":27,"codeName":"IndexNotFound"}

Cleaning up text indexes require a little bit of work.

db.collection.createIndex(
   {
     subject: "text",
     comments: "text"
   },
   {
     name: "MyTextIndex"
   }
)

db.collection.dropIndex("MyTextIndex")

[flovilmart]

I would not bother with removal for a v1. Creation is already supported for geo points, username etc... so we’d ‘just’ have to enhance the support in a more generic way

[montymxb]

Agreed, if no there's support for removing the other 'special' types it would be best to later integrate proper cleanup on not just this but all others as well.

[montymxb]

@dplewis can you clean up the error message? The underlying db should be of no consequence to the sdk, and the error messages should reflect that.

It should be throwing aParse.Error with a clear message, clarified to something like the message in the existing error MongoError. It could even be the same message, but it should not just be a dump of an internal system error object. Regardless of what the underlying DB the exception should be uniform. Including mongo specific stuff in a response doesn't really work that way.

Beyond all that, if you look at all the exception related testing in ParseQueryTest (and throughout the rest of the test suite) you'll see that the expected exception messages are just the messages, not the actual dump of a whole object like MongoError. It would be best to keep this the same way rather than breaking convention.

This also brings up a point regarding the error code that is returned. I haven't given it a look but if there's no appropriate error code we may have to introduce a new one. Something that can be looked up in the docs that clarifies an index must be created first for a given field.

[dplewis]

@montymxb I'll submit a PR later on parse-server to catch the mongo error. For now should I assume these test will fail?

[montymxb]

Yeah, this will have to hold until adding indexes is factored in for one, but after that you can handle the error. Once both those are squared away I'll go over this and we'll get this added in once it's all good 👍 .

Also how's #3944 looking for parse-server @flovilmart ? It would be nice to get that wrapped up before we start moving too far away from it.

[flovilmart]

I'll review before the end of the weekend! And merge obviously!

[montymxb]

@dplewis didn't realize that this got merged in already, awesome! I would run the tests again but I don't see anything server-side for setting up text indexes. Any news on that front? I can always drop in and provide a PR for adding an index server-side when I have some time.

[dplewis]

@montymxb Text Indexes aren't in there yet. Feel free to a PR.

[dplewis]

parse-community/parse-server#4081

I opened a PR for text indexes. It only supports one text index however. This is a quick solution until the schema gets updated.

[montymxb]

@dplewis I saw! Nice job man, internally we were talking about putting that in sometime soon but you may have beaten us to it! We'll have to figure out the multi-index issue, I think it's multiple indexes in one field, or the other way around, but I'm not actually not too sure...

[montymxb]

Tests actually look ok, minus what appears to be an issue regarding the push status tests. Looks like it could be timing related, but I'll take a look and see what's going on. Once that's resolved we can proceed with this.

[montymxb]

@dplewis this all looks good to go! Sorry for the delay again. The single failure in the latest run is something else independent of this, related to push status handling in the most recent version of parse server. I'll be addressing that in a separate PR shortly to verify it, and then I'll come back to add this in afterwards.

[montymxb]

Thanks again for the contribution! The one little issue we had has been rectified in #350, which was merged right before this.

[montymxb]

@dplewis when you have a moment can you open up a PR at the docs for this newly added feature?