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

Updated Date Range to Follow Documentation When Assuming Missing Values 8x #113872

Merged

Conversation

john-wagster
Copy link
Contributor

Based on a review of the documentation here: https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-range-query.html. Our behavior at query and index time for date range queries was inconsistent.

This would lead to what was discovered here: #111484 where an indexed date_range field like the one shown below would not be returned by the exact same query shown below.

I attempted to fix this by changing the query parser to use DateMathParser which rounds appropriately to what's documented above. This changes missing information in date time values to be inline with the documentation often by 1 millisecond (such as the time information in the below example).

A few concerns worth discussing with this are where this should land in terms of releases and impacts to customer who may have assumptions about how date range has been working up to this point. Opening this as a discussion with some caution around putting it in until we get appropriate feedback.

query example
PUT /test_index1?pretty
{
    "mappings" : {
        "properties" : {
            "opening_dates": {
              "type": "date_range",
              "format": "yyyy-MM-dd"
            }
        }
    }
}

PUT test_index1/_doc/7
{
    "opening_dates": [
    {
      "gte": "2014-09-18",
      "lte": "2016-12-18"
    }
  ]
}

GET test_index1/_search
{
  "explain": true, 
  "query": {
    "bool": {
      "must": [
        {
          "range": {
            "opening_dates": {
              "gte": "2014-09-18",
              "lte": "2016-12-18",
              "relation": "contains",
              "format": "yyyy-MM-dd"
            }
          }
        }
      ]
    }
  }
}

Copy link
Contributor

github-actions bot commented Oct 1, 2024

Documentation preview:

@john-wagster john-wagster added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Oct 1, 2024
@elasticsearchmachine elasticsearchmachine merged commit 8a8ad1b into elastic:8.x Oct 1, 2024
16 checks passed
@john-wagster john-wagster deleted the date-range-contains-fix-8x branch October 1, 2024 15:42
elasticsearchmachine pushed a commit that referenced this pull request Oct 1, 2024
…es 815 (#113882)

* updated rangetype to be more inline with the docs (https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-range-query.html) and added tests to reflect as much (#113872)

* moved mapper feature because test features don't exist yet
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) backport >bug v8.15.3 v8.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants