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

Sorting minimongo documents by a field of an explicitly specified array element in ascending order fails #2439

Closed
sschmitz opened this Issue Aug 26, 2014 · 2 comments

Comments

Projects
None yet
3 participants
@sschmitz

sschmitz commented Aug 26, 2014

A Minimongo query with a sort specifier like { 'arrayField.0.sortField': 1 } returns the matched documents in their natural order, while the same specifier with descending order works as expected. The issue is demonstrated in https://github.com/sschmitz/minimongo-array-sorting-bug/; running meteor and looking at the page is enough. The same sort specifier works as expected in a regular mongo shell.

I've only looked at the code rather quickly, but the reason seems to be that the lookup function (returned by makeLookupFunction) first checks the literal integer index, and then also does a branching search. When the key '0' is not present in an element of arrayField, the lookupRest function will return a branch with a value of undefined, which is appended to the results of the top-level lookup function. In Minimongo.Sorter.prototype._getMinKeyFromDoc, where the branches are passed via Minimongo.Sorter.prototype._generateKeysFromDoc, a key with a value of undefined is chosen over any other keys for sort specifiers with ascending order, while it is not chosen in the case of descending order. If no document in the collection contains an object with a '0' key in its arrayField, the chosen minimum key will have a value of undefined for every document, so no sorting will occur.

My intuition would be that to fix this, either a branching search should only be performed when the next part of the sort specifier is non-numeric, or to not have lookupRest return undefined when the lookup function is created by Minimongo.Sorter. However, I don't know about the implications of changing the behavior of the lookup function, nor do I know what matches the MongoDB behavior best in other cases.

glasser added a commit that referenced this issue Sep 4, 2014

@glasser

This comment has been minimized.

Member

glasser commented Sep 4, 2014

Good catch. Sadly I do recall that a lot of the complexity here was actually necessary for making the implementation be correct. For example, the selector {'a.1': 2} actually does match {a: [0, {1: 2}, 3]} in mongod (both 2.4 and 2.6).

Take a look at 0453d45

Running meteor test-packages minimongo shows a bunch of selector_compiler tests failing (and lookup tests, but that's testing an internal interface; testing the actual top-level match code for compatibility is what matters).

Perhaps in the specific context of sort selectors we should do what I have there? Yeah, I think that might make sense. I think maybe sort selectors should never branch at a numeric index. That seems familiar.

@glasser glasser closed this in 6432b49 Sep 4, 2014

glasser added a commit that referenced this issue Sep 5, 2014

@cesarve77

This comment has been minimized.

cesarve77 commented Jan 24, 2017

Same error Meteor 1.4 but for "a.b.0.c"
I made this simple repo for reproduce the bug
https://github.com/cesarve77/minimongo-sort-bug

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment