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 no longer supports unmapped_type for _score field #17392

Closed
Bargs opened this issue Mar 29, 2016 · 9 comments
Closed

Sort no longer supports unmapped_type for _score field #17392

Bargs opened this issue Mar 29, 2016 · 9 comments
Labels
discuss :Search/Search Search-related issues that do not fall into other categories

Comments

@Bargs
Copy link

Bargs commented Mar 29, 2016

Elasticsearch version: master

JVM version: 1.8.0_60

OS version: OSX

Description of the problem including expected versus actual behavior:

Issuing a query with a sort on the _score field with unmapped_type set will return a 400 with the message: [_score] failed to parse field [unmapped_type]. This used to work up until very recently, because Kibana sends unmapped_type: boolean by default with searches from the Discover tab. I'm wondering if this was a regression or an intentional change, because I can't find any mention of it in any GH issue or PR. unmapped_type does still work for regular fields, so I'm not sure if this is just _score, or any meta field.

Steps to reproduce:

  1. POST a search with the following body {"sort":[{"_score":{"order":"desc","unmapped_type":"boolean"}}]}
  2. Observe the 400 error

Kibana issue I created for more info about how we use unmapped_type: elastic/kibana#6698

@jpountz
Copy link
Contributor

jpountz commented Mar 30, 2016

It seems to be a side effect of the query refactoring efforts since all the request parsing logic got rewritten.

I am tempted to consider that the bug was to accept this parameter when sorting on _score since the _score is available on all shards and is not "mapped" anyway. Other sort options like script sort or geo-distance sort do not support this option either.

@Bargs
Copy link
Author

Bargs commented Mar 30, 2016

@jpountz are there any other meta fields that are sortable but don't support unmapped_type?

@cbuescher
Copy link
Member

Yes, @jpountz is right, this is a side effect of the refactoring. I just checked the old parser logic. While it parsed the "unmapped_type" parameter, it silentely ignored it, so I think the current behaviour of throwing an error is better. The only paramter that sort on _score now accepts is "order". All other metadata fields should accept the same parameters as any other field, however I'm not sure how useful they are in some cases. Sorting on _doc will accept all parameters as any other field but ignore them except for "order".

We can restore the previous behaviour of silently ignoring paramters like "missing", "unmapped_type" or "mode", but I think the current solution of treating it separately is better. Maybe @MaineC has something to add.

@MaineC
Copy link

MaineC commented Mar 31, 2016

+0 for keeping the current behaviour, it also seems more in line with changes we made to other parsers where we added exceptions e.g. to ambiguous parameters.

@cbuescher
Copy link
Member

@Bargs it it okay for you if we leave the current behaviour? @clintongormley any opinion on whether we should mark this as breaking in the docs or any of the migration guides? As mentioned above, the setting never had any effect for "_score", we only silently ignored it so far.

@Bargs
Copy link
Author

Bargs commented Mar 31, 2016

@cbuescher yeah, I think it's reasonable to leave it as is. I've committed a fix to Kibana which only required a small change. I would recommend marking it as a breaking change since it does have the potential to cause issues.

@clintongormley
Copy link
Contributor

@cbuescher this was undocumented and I wouldn't mark it as breaking. The one question I have is whether _doc should accept order? Seems to defeat the object of using _doc, no?

@clintongormley clintongormley added >docs General docs changes :Search/Search Search-related issues that do not fall into other categories discuss and removed >docs General docs changes labels Apr 4, 2016
@cbuescher
Copy link
Member

@clintongormley I agree there are no real use cases, we could also ignore it I guess. On the java API side, if we would want to also throw errors when "_doc" is used with "missing", "unmapped_type" etc.. rather than silently ignore it, we would need to add another builder class (like ScoreSortBuilder) for it. Not sure if thats worth the effort.

@clintongormley
Copy link
Contributor

@cbuescher i'm fine with leaving as is

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss :Search/Search Search-related issues that do not fall into other categories
Projects
None yet
Development

No branches or pull requests

5 participants