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

sort order doesn't work #14

Merged
merged 1 commit into from
Mar 12, 2013
Merged

Conversation

chadkouse
Copy link

I can't tell if it's something new in 0.90 or if it's just a documentation error but they show using both "order" and "sort_order" to control the order of the sort. Turns out at least in pre 0.90 "sort_order" doesn't work, only "order" will work

@polyfractal
Copy link
Owner

Hmm, it's either a documentation error or only applies to the new nested, multi-valued docs. In the Java source they only look at "order":

[...]
else if ("order".equals(innerJsonName)) {
  if ("asc".equals(parser.text())) {
    reverse = SCORE_FIELD_NAME.equals(fieldName);
  } else if ("desc".equals(parser.text())) {
    reverse = !SCORE_FIELD_NAME.equals(fieldName);
  }
}
[...]

If you add a pull request I'll merge/tag/push. I'm following up with the ES devs to see if this is an error or just specific to multi-value.

Thanks, and sorry for letting this slip through. My unit tests only check for successful queries...I need to spend some time working up better test environments so query results themselves can be tested (e.g. make sure things are sorting correctly).

@chadkouse
Copy link
Author

i have a change ready.. just about to submit the PR

Chad Kouse

On Tuesday, March 12, 2013 at 2:57 PM, Zachary Tong wrote:

Hmm, it's either a documentation error or only applies to the new nested, multi-valued docs. In the Java source (https://github.com/elasticsearch/elasticsearch/blob/master/src/main/java/org/elasticsearch/search/sort/SortParseElement.java#L137) they only look at "order":
[...] else if ("order".equals(innerJsonName)) { if ("asc".equals(parser.text())) { reverse = SCORE_FIELD_NAME.equals(fieldName); } else if ("desc".equals(parser.text())) { reverse = !SCORE_FIELD_NAME.equals(fieldName); } } [...]

If you add a pull request I'll merge/tag/push. I'm following up with the ES devs to see if this is an error or just specific to multi-value.
Thanks, and sorry for letting this slip through. My unit tests only check for successful queries...I need to spend some time working up better test environments so query results themselves can be tested (e.g. make sure things are sorting correctly).


Reply to this email directly or view it on GitHub (#14 (comment)).

polyfractal added a commit that referenced this pull request Mar 12, 2013
Fix sort order with, 'order' instead of 'sort_order'
@polyfractal polyfractal merged commit b4bce4c into polyfractal:master Mar 12, 2013
@polyfractal
Copy link
Owner

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

Successfully merging this pull request may close these issues.

2 participants