-
Notifications
You must be signed in to change notification settings - Fork 41
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
current sorting on multiple values is broken by design #12
Comments
Currently, the version of V8 in Node keeps track of key order, so it would work for now, but I'd rather not rely on that too much. originally, we went with that format for familiarity from Mongo syntax. But looks like they actually want you to use compound indexes for multiple sort criteria (see http://docs.mongodb.org/manual/tutorial/sort-results-with-indexes/) What your'e suggesting is good-- but I think we'd need to make some changes in core. It could be done in a backwards-compatible way (if you pass in an object, it'll do it the current way-- if you pass in an array, it'll do it the new way) Either way, to make it easier to standardize the API for multi-criteria sort across all of our adapters, I think it would make sense to pull out a separate interface, i.e. @postsql @particlebanana @sgress454 What do you guys think? |
Any update on this? |
ECMAScript spec says : i think using a V8 current specificity is not a good way. @mikermcneil you are right,using interface to detect adapter functionality but is there some doc about adapter function and interface spec ? |
Agreed, balderdashy/waterline#694 should fix this.
The best source and probably a bit dated is sails-docs - adapter-specification. There are other references in waterline-docs - adapters that may help. |
@atiertant @dmarcelino let's add this in the 0.11 milestone on Waterline. |
balderdashy/waterline#694 and this are now added to milestone |
the test says:
it('should sort records using multiple sort criteria, with first name asc', function(done) {
User.find({ where: { type: 'sort test multi' }, sort: { last_name: 1, first_name: 1 } }
but as there is no defined order for object fields (hash/dictionary keys), then you can not meaningfully specify multi-key sort order using dictionaries. It has to be an Array, so the test (and implementations) should do:
User.find({ where: { type: 'sort test multi' }, sort: [{ last_name: 1}, {first_name: 1 }] }
to guarantee that result is sorted on last_name first and then on first_name
The text was updated successfully, but these errors were encountered: