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

Range queries do not support time_zone with epoch fields #22621

Closed
brandond opened this issue Jan 13, 2017 · 9 comments
Closed

Range queries do not support time_zone with epoch fields #22621

brandond opened this issue Jan 13, 2017 · 9 comments
Labels
>enhancement good first issue low hanging fruit help wanted adoptme :Search Foundations/Mapping Index mappings, including merging and defining field types Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch

Comments

@brandond
Copy link

brandond commented Jan 13, 2017

Elasticsearch version: 5.1.1

Plugins installed: search-guard-ssl

JVM version: openjdk version "1.8.0_111"

OS version: Amazon Linux AMI release 2016.09

Description of the problem including expected versus actual behavior:

The documentation says that dates in ranges can be converted from different time zones using the time_zone parameter. However, specifying a non-UTC time_zone along with the epoch_millis or epoch_second formatter (or not specifying a formatter and simply providing a numeric timestamp) fails because the EpochTimeParser is hardcoded to return failure if the time zone is not UTC:
https://github.com/elastic/elasticsearch/blob/master/core/src/main/java/org/elasticsearch/common/joda/Joda.java#L337

Steps to reproduce:

  1. Execute the following search against a Logstash index:
{
  "query": {
    "range": {
      "@timestamp": {
        "from": "2011-01-13T11:00:00",
        "time_zone": "America/Los_Angeles",
        "format": "strict_date_optional_time"
      }
    }
  }
}
  1. Execute the same search using epoch_millis:
{
  "query": {
    "range": {
      "@timestamp": {
        "from": 1484334000000,
        "time_zone": "America/Los_Angeles",
        "format": "epoch_millis"
      }
    }
  }
}
  1. Execute the same search using epoch_second:
{
  "query": {
    "range": {
      "@timestamp": {
        "from": 1484334000,
        "time_zone": "America/Los_Angeles",
        "format": "epoch_second"
      }
    }
  }
}

Provide logs (if relevant):
The first search (using a datetime string) will work fine. The two searches that use an epoch timestamp fail with a parse exception:

{
    "error": {
        "root_cause": [
            {
                "type": "parse_exception",
                "reason": "failed to parse date field [1484334000000] with format [epoch_millis]"
            }
        ],
        "type": "search_phase_execution_exception",
        "reason": "all shards failed",
        "phase": "query",
        "grouped": true,
        "failed_shards": [
            {
                "shard": 0,
                "index": "logstash-2016.12.07",
                "node": "v13nGpFcQrC9EUTjRh8FGA",
                "reason": {
                    "type": "parse_exception",
                    "reason": "failed to parse date field [1484334000000] with format [epoch_millis]",
                    "caused_by": {
                        "type": "illegal_argument_exception",
                        "reason": "Parse failure at index [0] of [1484334000000]"
                    }
                }
            }
        ],
        "caused_by": {
            "type": "parse_exception",
            "reason": "failed to parse date field [1484334000000] with format [epoch_millis]",
            "caused_by": {
                "type": "illegal_argument_exception",
                "reason": "Parse failure at index [0] of [1484334000000]"
            }
        }
    },
    "status": 400
}
@jpountz
Copy link
Contributor

jpountz commented Jan 15, 2017

It is not clear to me whether you are asking for a better error message or for a different behaviour? I think the only alternative would be to ignore the time zone in those cases, but it could also look trappy to some users that we allow passing a time zone just to ignore it?

@clintongormley
Copy link
Contributor

it could also look trappy to some users that we allow passing a time zone just to ignore it?

+1

i think the behaviour is correct as it stands

@brandond
Copy link
Author

brandond commented Jan 16, 2017

@jpountz I understand why you would want to force epoch timestamps to only support UTC. However, at the very least a documentation update would be nice, and/or a better error message indicating that TZ cannot be used with epoch timstamps. As it currently stands, the error message does not in any way indicate the cause of failure - it claims to be a parser failure, when in fact it was parsed just fine, but used with an unsupported combination of parameters. You have to go read a comment in the code to figure out why its REALLY failing, something that is totally unexpected if you've been reading the docs.

@clintongormley
Copy link
Contributor

Agreed, a better error message would help

@njlawton
Copy link

Hi I am new to this project could I pick this issue up as my first fix to get a bit more familiar with the project?

@dakrone
Copy link
Member

dakrone commented Mar 10, 2017

Hi @njlawton that sounds great, this is labeled as "adoptme" so feel free to tackle it!

@njlawton
Copy link

njlawton commented Mar 20, 2017

Hi sorry I will no longer be working on this issue

@sneivandt
Copy link
Contributor

I would like to pick up this issue as my first contribution. I plan on resolving this by changing the response to be something like the following.

time_zone must be UTC when using format [epoch_millis]

@brandond
Copy link
Author

@sneivandt Any particular reason you're adding the check in RangeQueryBuilder, as opposed to somewhere closer to the Joda EpochTimeParser class that actually checks for non-UTC time zones? Seems like there are a lot of other places that might potentially ask EpochTimeParser to handle a non-UTC timestamp.

cbuescher pushed a commit that referenced this issue Mar 28, 2017
Change the error response when using a non UTF timezone for range queries with epoch_millis
or epoch_second formats to an illegal argument exception. The goal is to provide a better 
explanation of why the query has failed. The current behavior is to respond with a parse exception.

Closes #22621
@clintongormley clintongormley added :Search Foundations/Mapping Index mappings, including merging and defining field types and removed :Dates labels Feb 13, 2018
@javanna javanna added the Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch label Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement good first issue low hanging fruit help wanted adoptme :Search Foundations/Mapping Index mappings, including merging and defining field types Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch
Projects
None yet
Development

No branches or pull requests

7 participants