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

$regex throws incorrect error with an empty string argument #1874

Closed
wants to merge 10 commits into
base: devel
from

Conversation

Projects
None yet
2 participants
@mizzao
Contributor

mizzao commented Feb 28, 2014

In 0.7.1.2, a query such as Foo.find({field: {$regex: "", $options: "i"}}) leads to the following error:

Exception from Deps recompute: Error: $options needs a $regex

This is because the argument checking logic doesn't account for $regex possibly being an empty string. In Mongo, the same query would return all documents for which the field exists.

Discovered by @geekyme in mizzao/meteor-autocomplete#15

@mizzao

This comment has been minimized.

Contributor

mizzao commented Feb 28, 2014

This may be a bit off topic, but is there a way to suppress all those merge commits as I am pulling from upstream? I grabbed the latest devel and only changed one file.

@glasser glasser closed this in 45db64d Feb 28, 2014

@glasser

This comment has been minimized.

Member

glasser commented Feb 28, 2014

Thanks, fixed (using _.has which we use more often in our codebase and with a test).

@mizzao

This comment has been minimized.

Contributor

mizzao commented Feb 28, 2014

Thank you for the quick fix!

@mizzao mizzao deleted the scienceathome:regex-operator-fix branch Feb 28, 2014

@mizzao

This comment has been minimized.

Contributor

mizzao commented Mar 7, 2014

@glasser, it's too bad that $regex refuses to match against numbers now (https://github.com/meteor/meteor/blob/devel/packages/minimongo/selector.js#L206). This was really convenient for things like https://github.com/mizzao/meteor-autocomplete, since Javascript will easily coerce numbers to strings for comparison.

Is there any way we could have this available on the client side in a principled way? Or, since the Javascript RegExp is willing to test against numbers, can we just allow it to?

As an alternative, I'm considering sending both numeric and string valued fields for this, containing the same value, which would seem to be pretty inefficient.

@glasser

This comment has been minimized.

Member

glasser commented Mar 7, 2014

@mizzao Adding comments on unrelated issues isn't to best wait to raise an issue. That said, this change was made for Mongo consistency.

meteor:PRIMARY> db.x.insert({x: 42})
meteor:PRIMARY> db.x.find({x: /4/})
meteor:PRIMARY> 

dandv added a commit to mizzao/meteor-autocomplete that referenced this pull request May 3, 2014

Cosmetic changes to the demo
* set Meteor release back to 0.8.0 because Windows folks may not have 0.8.1.1
* use single quotes consistently
* Minimongo was patched for the $regex issue (meteor/meteor#1874) so update README
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment