Elasticsearch: Add support for querying on range fields in query_string and simple_query_string queries

Created on 8 Sep 2017  路  9Comments  路  Source: elastic/elasticsearch

Currently querying on range fields via query_string and simple_query_string queries results in an error indicating that we do not support this. Whereas querying these fields via the range queries just works as expected.

Adding support for this is trivial. One caveat is that there is no way to specify the relation (like in range query). Therefor if range fields are queries via query_string and simple_query_string then we should default to intersects relation.

:SearcSearch >enhancement good first issue help wanted

Most helpful comment

This is solved tangentially by https://github.com/elastic/elasticsearch/pull/26552 because the query_string uses the new signature of MappedFieldType#rangeQuery. As a side effect the query_string will default to intersects when building a range query on a range field.
simple_query_string does not handle range query (there is no syntax for it) so the only thing to do here would be to add unit tests for the query_string.
@arunagar is it something you want to contribute ? Adding unit tests would ensure that we don't break the support in future versions.

All 9 comments

i would like to pick this up as my first PR, do i have to assign this to myself to start working on it ?

@arunagar You can just open a PR and we can then work on get your change in. No one from Elastic is working on this yet as it hasn't been assigned yet.

This is solved tangentially by https://github.com/elastic/elasticsearch/pull/26552 because the query_string uses the new signature of MappedFieldType#rangeQuery. As a side effect the query_string will default to intersects when building a range query on a range field.
simple_query_string does not handle range query (there is no syntax for it) so the only thing to do here would be to add unit tests for the query_string.
@arunagar is it something you want to contribute ? Adding unit tests would ensure that we don't break the support in future versions.

I will start on this and send a PR, i may take some time to get accustomed to the code, but i will work on this.

Hey i was looking at the query_string test for range, there is already existing test in here for range
https://github.com/elastic/elasticsearch/blob/c709b8d6ac7ef50a42c3f69f07132d2b0515f385/core/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java#L918

Is the intention to write a unit test with query_string, which makes sure that the relation is intersect ?

@arunagar this test is on a keyword field, what's missing is a query on a range field:
https://www.elastic.co/guide/en/elasticsearch/reference/current/range.html

@jimczi Is there any problem if I work on the needed test?

Range fields are in a separate module, so rather than adding a test to QueryStringQueryBuilderTests I'm going to create a new AbstractQueryTestCase extension in mapper-extras. Does that make sense @jpountz ?

@romseygeek :+1:

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Praveen82 picture Praveen82  路  3Comments

martijnvg picture martijnvg  路  3Comments

brwe picture brwe  路  3Comments

makeyang picture makeyang  路  3Comments

clintongormley picture clintongormley  路  3Comments