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.

Sign up for GitHub

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

Jump to bottom

simple_query_string with fields: ["*"] can result in an error #39577

Closed
jakelandis opened this issue Mar 1, 2019 · 5 comments · Fixed by #42301
Closed

simple_query_string with fields: ["*"] can result in an error #39577

jakelandis opened this issue Mar 1, 2019 · 5 comments · Fixed by #42301
Assignees
@matriv @jimczi
Labels
>enhancement :Search/Search Search-related issues that do not fall into other categories

Comments

@jakelandis
Copy link
Contributor

Reproduction:

PUT _template/foo
{
  "settings": {
    "index": {
      "number_of_shards": "1",
      "number_of_replicas": "0"
    }
  },
  "index_patterns": [
    "foo"
  ],
  "mappings": {
    "dynamic": "strict",
    "properties": {
      "kw": {
        "type": "keyword"
      },
      "b": {
        "type": "boolean"
      }
    }
  }
}

DELETE foo
PUT foo/_create/1
{
  "kw": "fizzbuzz",
  "b": true
}

GET foo/_search
{
  "query": {
    "simple_query_string": {
      "query": "fizzbuzz",
      "fields": ["*"]
    }
  }
}

Results in

[2019-03-01T11:07:38,419][DEBUG][o.e.a.s.TransportSearchAction] [macd.ex9.net] All shards failed for phase: [query]
org.elasticsearch.index.query.QueryShardException: failed to create query: {
  "simple_query_string" : {
    "query" : "fizzbuzz",
    "fields" : [
      "*^1.0"
    ],
    "flags" : -1,
    "default_operator" : "or",
    "analyze_wildcard" : false,
    "auto_generate_synonyms_phrase_query" : true,
    "fuzzy_prefix_length" : 0,
    "fuzzy_max_expansions" : 50,
    "fuzzy_transpositions" : true,
    "boost" : 1.0
  }
}
	at org.elasticsearch.index.query.QueryShardContext.toQuery(QueryShardContext.java:309) ~[elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at org.elasticsearch.index.query.QueryShardContext.toQuery(QueryShardContext.java:292) ~[elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at org.elasticsearch.search.SearchService.parseSource(SearchService.java:754) ~[elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at org.elasticsearch.search.SearchService.createContext(SearchService.java:607) ~[elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at org.elasticsearch.search.SearchService.createAndPutContext(SearchService.java:582) ~[elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at org.elasticsearch.search.SearchService.executeQueryPhase(SearchService.java:385) ~[elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at org.elasticsearch.search.SearchService.access$100(SearchService.java:123) ~[elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at org.elasticsearch.search.SearchService$2.onResponse(SearchService.java:357) [elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at org.elasticsearch.search.SearchService$2.onResponse(SearchService.java:353) [elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at org.elasticsearch.search.SearchService$4.doRun(SearchService.java:1068) [elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at org.elasticsearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:37) [elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at org.elasticsearch.common.util.concurrent.TimedRunnable.doRun(TimedRunnable.java:41) [elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at org.elasticsearch.common.util.concurrent.ThreadContext$ContextPreservingAbstractRunnable.doRun(ThreadContext.java:751) [elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at org.elasticsearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:37) [elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) [?:?]
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) [?:?]
	at java.lang.Thread.run(Thread.java:834) [?:?]
Caused by: java.lang.IllegalArgumentException: Can't parse boolean value [fizzbuzz], expected [true] or [false]
	at org.elasticsearch.index.mapper.BooleanFieldMapper$BooleanFieldType.indexedValueForSearch(BooleanFieldMapper.java:166) ~[elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at org.elasticsearch.index.mapper.TermBasedFieldType.termQuery(TermBasedFieldType.java:53) ~[elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at org.elasticsearch.index.mapper.BooleanFieldMapper$BooleanFieldType.termQuery(BooleanFieldMapper.java:114) ~[elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at org.elasticsearch.index.search.MatchQuery$MatchQueryBuilder.lambda$newTermQuery$1(MatchQuery.java:526) ~[elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at org.elasticsearch.index.search.MatchQuery$MatchQueryBuilder.newTermQuery(MatchQuery.java:529) ~[elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at org.elasticsearch.index.search.MatchQuery.parse(MatchQuery.java:251) ~[elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at org.elasticsearch.index.search.MultiMatchQuery.buildFieldQueries(MultiMatchQuery.java:101) ~[elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at org.elasticsearch.index.search.MultiMatchQuery.parse(MultiMatchQuery.java:69) ~[elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at org.elasticsearch.index.search.SimpleQueryStringQueryParser.newDefaultQuery(SimpleQueryStringQueryParser.java:118) ~[elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at org.apache.lucene.queryparser.simple.SimpleQueryParser.consumeToken(SimpleQueryParser.java:415) ~[lucene-queryparser-8.0.0-snapshot-83f9835.jar:8.0.0-snapshot-83f9835 83f9835a47a00a2ec58a4cf5fc0d492497cf7898 - jpountz - 2019-01-21 13:06:32]
	at org.apache.lucene.queryparser.simple.SimpleQueryParser.parseSubQuery(SimpleQueryParser.java:216) ~[lucene-queryparser-8.0.0-snapshot-83f9835.jar:8.0.0-snapshot-83f9835 83f9835a47a00a2ec58a4cf5fc0d492497cf7898 - jpountz - 2019-01-21 13:06:32]
	at org.apache.lucene.queryparser.simple.SimpleQueryParser.parse(SimpleQueryParser.java:156) ~[lucene-queryparser-8.0.0-snapshot-83f9835.jar:8.0.0-snapshot-83f9835 83f9835a47a00a2ec58a4cf5fc0d492497cf7898 - jpountz - 2019-01-21 13:06:32]
	at org.elasticsearch.index.query.SimpleQueryStringBuilder.doToQuery(SimpleQueryStringBuilder.java:431) ~[elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at org.elasticsearch.index.query.AbstractQueryBuilder.toQuery(AbstractQueryBuilder.java:99) ~[elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at org.elasticsearch.index.query.QueryShardContext.lambda$toQuery$1(QueryShardContext.java:293) ~[elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	at org.elasticsearch.index.query.QueryShardContext.toQuery(QueryShardContext.java:305) ~[elasticsearch-7.0.0-beta1.jar:7.0.0-beta1]
	... 16 more

This is a contrived example from a more complex query.

The underlying error makes sense, but the doc states "will never throw an exception, and discards invalid parts of the query" , and not sure if this qualifies an invalid part of the query.

The query works if you simply remove "fields": ["*"] , so there is a simple workaround, but then you are implicitly depending on index.query.default_field which may not be what you want.
@jakelandis jakelandis added the :Search/Search Search-related issues that do not fall into other categories label Mar 1, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

@jpountz
Copy link
Contributor

jpountz commented Mar 1, 2019

I had a discussion with @jakelandis about this one. The confusion comes from the fact that we recommend using * as a default field in replacement of all_fields:true. This behaves the same on the query_string query which implicitly sets the lenient flag to true when the default field is *. However the simple_query_string doesn't, which is probably a bug?

The other inconsistency is that we automatically set lenient to true when default_field is * but not when fields is [ "*" ] which is a bit confusing too IMO as I would expect both default_field: * and fields: [ * ] to behave the same as a user.

@jakelandis
Copy link
Contributor Author

Retested example above with "lenient": true and all works well.

GET foo/_search
{
  "query": {
    "simple_query_string": {
      "query": "fizzbuzz",
      "fields": ["*"],
      "lenient": true
    }
  }
}

👍

@alexwizp alexwizp mentioned this issue Mar 4, 2019
Merged
7 tasks
@jimczi
Copy link
Contributor

jimczi commented Mar 5, 2019 •
edited
Loading

We discussed this issue internally and agreed that we should solve the discrepancy by setting lenient to true automatically when all fields are targeted in these queries (no matter if it's set in fields, default_field or the index.query.default_field).

@jimczi jimczi self-assigned this Mar 5, 2019
@matriv matriv assigned matriv and jimczi and unassigned jimczi May 21, 2019
matriv added a commit to matriv/elasticsearch that referenced this issue May 21, 2019
@matriv
Allow fields to be set to *
3fa53ce 
Allow for SimpleQueryString, QueryString and MultiMatchQuery
to set the `fields` parameter to the wildcard `*`. If so, set
the leniency to `true`, to achieve the same behaviour as from the
`"default_field" : "*" setting.

Closes: elastic#39577
@matriv matriv mentioned this issue May 21, 2019
Merged
matriv added a commit that referenced this issue May 23, 2019
@matriv
Allow fields to be set to * (#42301)
e75ff0c 
Allow for SimpleQueryString, QueryString and MultiMatchQuery
to set the `fields` parameter to the wildcard `*`. If so, set
the leniency to `true`, to achieve the same behaviour as from the
`"default_field" : "*" setting.

Furthermore,  check if `*` is in the list of the `default_field` but
not necessarily as the 1st element.

Closes: #39577
matriv added a commit that referenced this issue May 23, 2019
@matriv
Allow fields to be set to * (#42301)
0777223 
Allow for SimpleQueryString, QueryString and MultiMatchQuery
to set the `fields` parameter to the wildcard `*`. If so, set
the leniency to `true`, to achieve the same behaviour as from the
`"default_field" : "*" setting.

Furthermore,  check if `*` is in the list of the `default_field` but
not necessarily as the 1st element.

Closes: #39577
(cherry picked from commit e75ff0c)
@matriv
Copy link
Contributor

matriv commented May 23, 2019

Backported to 7.x with 0777223

gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this issue May 27, 2019
@matriv
Allow fields to be set to * (elastic#42301)
c9e39ef 
Allow for SimpleQueryString, QueryString and MultiMatchQuery
to set the `fields` parameter to the wildcard `*`. If so, set
the leniency to `true`, to achieve the same behaviour as from the
`"default_field" : "*" setting.

Furthermore,  check if `*` is in the list of the `default_field` but
not necessarily as the 1st element.

Closes: elastic#39577
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@matriv matriv

@jimczi jimczi

Labels
>enhancement :Search/Search Search-related issues that do not fall into other categories
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Allow fields to be set to * matriv/elasticsearch
5 participants
@jpountz @jakelandis @matriv @elasticmachine @jimczi