elastic / elasticsearch

Free and Open Source, Distributed, RESTful Search Engine
https://www.elastic.co/products/elasticsearch
Other
990 stars 24.82k forks source link

Range query on date fields causing issues in 5.x #23014

Closed jakommo closed 8 months ago

jakommo commented 7 years ago

Indexing a date using java.util.Date and query it again using java.util.Date results in:

org.elasticsearch.transport.RemoteTransportException: [node01][10.0.0.1:9300][indices:data/read/search[phase/query]] 
Caused by: org.elasticsearch.ElasticsearchParseException: failed to parse date field [Tue Jan 31 11:44:25 CET 2017] with format [strict_date_optional_time||epoch_millis] 
at org.elasticsearch.common.joda.DateMathParser.parseDateTime(DateMathParser.java:213) ~[elasticsearch-5.2.0.jar:5.2.0] 
at org.elasticsearch.common.joda.DateMathParser.parse(DateMathParser.java:66) ~[elasticsearch-5.2.0.jar:5.2.0] 
at org.elasticsearch.index.mapper.DateFieldMapper$DateFieldType.parseToMilliseconds(DateFieldMapper.java:305) ~[elasticsearch-5.2.0.jar:5.2.0] 
at org.elasticsearch.index.mapper.DateFieldMapper$DateFieldType.isFieldWithinQuery(DateFieldMapper.java:337) ~[elasticsearch-5.2.0.jar:5.2.0] 

It seem to be related to org.elasticsearch.common.io.stream.StreamOutput, line 577:

writers.put(Date.class, (o, v) -> {
            o.writeByte((byte) 12);
            o.writeLong(((Date) v).getTime());
        });

But the following used on the server side: org.elasticsearch.index.mapper.DateFieldMapper, line 292

        public long parseToMilliseconds(Object value, boolean roundUp,
                @Nullable DateTimeZone zone, @Nullable DateMathParser forcedDateParser, QueryRewriteContext context) {
            DateMathParser dateParser = dateMathParser();
            if (forcedDateParser != null) {
                dateParser = forcedDateParser;
            }

            String strValue;
            if (value instanceof BytesRef) {
                strValue = ((BytesRef) value).utf8ToString();
            } else {
                strValue = value.toString();
            }
            return dateParser.parse(strValue, context::nowInMillis, roundUp, zone);
        }

strValue is created by Date.toString which creates the unparsable string.

As a workaround providing a long of the time in millis works.

Discussed this internally with @markharwood and he brought up the idea if we should tighten the API up and have a DateRangeQueryBuilder like .net does?

martinscholz83 commented 7 years ago

Hi @jakommo, i assume you actually use the RangeQueryBuilder for your query. If it's ok i would start and try to implement such a QueryBuilder if this is needed?

jakommo commented 7 years ago

@markharwood or @clintongormley mind commenting on this? I'm not deep enough into this.

markharwood commented 7 years ago

I've not used the Java API for date queries personally but when I looked at some of the problems being reported I could see people having mismatched assumptions about what to pass to the generic RangeQueryBuilder which only takes anonymous java Object instances for range values. For date ranges it is unclear if users could/should pass: 1) Long objects representing time in millis? 2) java.util.Date ? 3) java.sql.Date? 4) java.util.Calendar? 5) jodatime objects?

There are inconsistent results/errors with these choices so the proposal is we tighten up the contract for Date range queries by having a dedicated DateRangeQueryBuilder class that only accepts one of these forms as the way to do date range queries. I assume it would be a simple lightweight class/subclass that is only used to enforce correct typing of params. I don't profess to be an expert in dates/timezones so I'd leave it for someone else to determine what is the safest choice of object type to use for defining the dates.

elasticmachine commented 6 years ago

Pinging @elastic/es-core-infra

dakrone commented 8 months ago

I'm going to close this one, as we've removed the high level rest client in favor of the Java client.