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

let "unique: true" ensure mongo index? #36

Closed
testbird opened this issue Dec 29, 2013 · 19 comments
Closed

let "unique: true" ensure mongo index? #36

testbird opened this issue Dec 29, 2013 · 19 comments

Comments

@testbird
Copy link

Meteors _ensureIndex could be used until the stable API comes along.
http://stackoverflow.com/questions/11392566/mongo-geospatial-index-and-meteor

@aldeed
Copy link
Collaborator

aldeed commented Dec 29, 2013

The docs say to combine it with ensureIndex, but I'm not sure I want to force that. If anyone else reading this has an opinion, feel free to weigh in.

@testbird
Copy link
Author

Unfortunately, I don't have experience to weigh in. Maybe it could be optional by setting something like "unique: true" vs. "unique: index".

@testbird
Copy link
Author

If the current implementation is rather something like a pre-check for duplicates,
maybe rename that to "dupcheck: true" and let "unique: true" be the option that
truely ensures uniqueness (ensuring an index if that is necessary).

@aldeed
Copy link
Collaborator

aldeed commented Dec 30, 2013

The main purpose was to be able to display client-side errors about uniqueness, so yes mostly a pre-check. Will consider.

@caseylutz
Copy link

Eric, strictly from a performance standpoint, pre-checking unique requires you to issue a query/fetch to check length and verify unique or not unique, correct? On the client that seems OK, I don't care about spending cycles on MiniMongo. On the server, though, it seems like it would make a lot of sense to use _ensureIndex and let Mongo determine the uniqueness constraint and throw exceptions as needed. Maybe this is what you're already doing?

I'm just envisioning a scenario where I might be importing 10,000 records from an external data system and poor Collection2 doubling the cost of effort to do so. Granted if speed was a real concern then one would use _collection.insert instead.

I like testbird's idea of a true | false | 'index' selection and if it's 'index' then let Collection2 handle the indexes. I think if I'm declaring schemas and calling out "uniqueness" it's a good way to avoid forgetting to apply unique constraints at the DB level.

2c.

@aldeed
Copy link
Collaborator

aldeed commented Dec 31, 2013

@caseylutz, good points. I think it does full finds on the server right now, so yes could be expensive. I never really intended that to be the final way it would work. An ideal situation would be if the Meteor folks add unique indexes to minimongo, and then we could try to find some way to tap into that error message to display the reactive simple-schema messages, etc.

Regarding true/false/'index', I guess what I'm trying to ponder is whether there are cases in which one would want unique: true but not want the server-side index? All I can think of is some sort of multi-key index scenario, but for multi-keys it would be undesirable to set unique: true, so that doesn't make sense.

I'm leaning toward calling _ensureIndex automatically when unique: true.

@caseylutz
Copy link

I totally agree. I like the simplicity of just having unique: true and being done with it. I figured there might be a reason I couldn't think of to allow a unique to be specified without forcing the indexes for... whatever reason. I can't think of any off the top of my head either. The only situation that possibly occurs to me is if, for whatever reason, the uniqueness constraint is only a "soft" constraint that's applicable to form validation and not a true constraint that the server or database need to be concerned with. The use cases for that are practically 0 as far as I can imagine.

Out of curiosity, if you go with the _ensureIndex route, do you think you will still be able to capture and bubble up validation-error messages if they occur on the server via autoForms, or would that end up being an edge case where the user may potentially not see any visible feedback for a failure?

@aldeed
Copy link
Collaborator

aldeed commented Dec 31, 2013

I think I would keep the find pre-check on the client, which would catch many uniqueness issues (depending on subscriptions), and then rely on _ensureIndex on the server, which would likely not have the normal reactive client-side feedback. Actually, currently none of the server-side validation results in client-side messages. This is generally fine because all of the checks are duplicated on the client, but in cases like this where subscriptions play a role in the validation, it would be a handy feature to validate on the server and react to that on the client. This is related to another issue that proposes allowing custom server-side validation with client reactivity (over DDP), but it would mean some significant rethinking because the client-side validation is synchronous right now.

The point about soft constraints is valid, but those could be achieved with custom validation. There's a pending enhancement to allow a separate "warning" schema for an autoform, and that would be another solution.

@caseylutz
Copy link

Yep that's definitely true. That problem you mention right there is one of the head-scratchers I've had going with the whole Meteor (as well as, to some extent, Collection2/AutoForm) validation scheme... There seems to be an assumption that clients are being handed lots of data about the universe and are generally self-sufficient in terms of validation and such. However, I find in my most of my applications clients are privvy to only a very small selection of data but their validations are heavily dependent on data they're not privileged to see.

Easiest example is picking a "username". I don't want users being published the usernames for every user on the site to validate, upon submit, whether or not that username is already taken (unique: true). Username is just a basic example... Maybe it's a security issue or maybe I have 2 million users and don't want 2 million names going down my potentially expensive wire. Validating it will be the server's job. However, if the server is going to silently throw an exception and the client is going to look at a form that just won't "go", then for that particular use case something like autoForm won't work; it'll have to be written by hand or pre-processed in a before hook with a meteor method.

@testbird
Copy link
Author

testbird commented Jan 1, 2014

Hy there, happy new year!

A reason not to want an index can be performance with high write loads. To avoid an index one would have to use another pattern involving findAndModify or an optimistic loop, but for this you simply don't specify "unique:true" on the collection, and implemnt/use that other pattern instead. So I think having unique:true ensure an index seems ok.

Concerning server side validation, one usefull pattern I think could be to use isSimulation to let (only) the clients add some extra "unconfirmed: true" field locally, and that then gets overwritten (deleted) again after the operation succceeded on the server and the confirmed document is propagated.
http://stackoverflow.com/questions/10082537/in-meteor-how-do-i-show-newly-inserted-data-as-greyed-out-until-its-been-confi

@testbird
Copy link
Author

testbird commented Jan 1, 2014

if the server is going to silently throw an exception and the client is going to look at a form that just won't "go", then for that particular use case something like autoForm won't work; it'll have to be written by hand or pre-processed in a before hook with a meteor method.

For user accounts, you may be able to make the relevant subset of input fields work similar to a search box (sending the changed text to the server every something ms), but only turning red if the search gets an exact hit (instead of displaying selections).

By now, maybe it is ok starting to base such a search on a mongo text index (instead of implementing own indexing)
https://github.com/Crenshinibon/spomet
http://stackoverflow.com/questions/14567856/full-text-search-with-meteor-js-and-mongodb
http://stackoverflow.com/questions/17159626/implementing-mongodb-2-4s-full-text-search-in-a-meteor-app

@testbird
Copy link
Author

testbird commented Jan 1, 2014

just saw this mentioned (without testing/checking implementation):

The Accounts.createUser method automatically returns an error if the username/email is duplicated.
http://stackoverflow.com/questions/19886120/how-i-catch-and-insert-meteor-error-alerts-from-meteor-methods-in-to-a-client-si

@aldeed
Copy link
Collaborator

aldeed commented Jan 8, 2014

Hey all, check out the referenced PR by @mquandalle. I think it's a pretty robust solution to indexing. The explanation in the readme is here:

https://github.com/mquandalle/meteor-collection2/blob/24fb25d7f4d9934b91a7f213e64918164a265cf0/README.md#mongodb-indexes

Any concerns?

@mquandalle
Copy link
Contributor

I should add a note that if a field has both index and unique options set, the package ensureIndex with the unique option of MongoDB.

@aldeed
Copy link
Collaborator

aldeed commented Jan 8, 2014

Right. I saw that in the code, but I didn't realize the readme section doesn't mention it. Should be updated.

@testbird
Copy link
Author

testbird commented Jan 8, 2014

Good separation of index and unique, thumbs up!
Thank you, looking forward to it. :)

@caseylutz
Copy link

Very nice; very elegant solution.

@aldeed
Copy link
Collaborator

aldeed commented Jan 14, 2014

The index option is released in 0.3.0 now.

@aldeed aldeed closed this as completed Jan 14, 2014
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

No branches or pull requests

4 participants