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

Zero milliseconds are suppressed from DateTime values during serialization in DateRange queries #6444

Closed
lekrus opened this issue Jun 11, 2022 · 2 comments
Labels
7.x Relates to a 7.x client version 8.x Relates to 8.x client version Category: Bug

Comments

@lekrus
Copy link

lekrus commented Jun 11, 2022

NEST/Elasticsearch.Net version:
7.13.2

Description of the problem including expected versus actual behavior:
The issue is a side-effect of #3899
When DateTime values without milliseconds are serialized, milliseconds part is not included and that will result in wrong results for Lt, Gt queries.

Related ticket is elastic/elasticsearch#62268

Steps to reproduce:

var dateWithoutMilliseconds = new DateTime(2022, 6, 11, 13, 54,  20, 0);
var serialized = Client.Serializer().SerializeToString(new SearchDescriptor<IDictionary>().AllIndices().Query(q => q
   .DateRange(dr=>dr
   .Field("test_field")
   .GreaterThan(dateWithoutMilliseconds)
)));
serialized.Should().Be("{\"query\":{\"range\":{\"test_field\":{\"gt\":\"2022-06-11T13:54:20.000\"}}}}");

Expected behavior
It's expected the date serialized with milliseconds part: 2022-06-11T13:54:20.000
Currently that part is suppressed in ToMinThreeDecimalPlaces method that uses FFFFFF custom format that suppresses zeros and outputs: 2022-06-11T13:54:20

And then if we search for documents where test_field>"2022-06-11T13:54:20" Elasticsearch will not return documents with test_fields == "2022-06-11T13:54:20.125" (as I understood internally the value of 2022-06-11T13:54:20.999999 is used if milliseconds are omitted)

@stevejgordon stevejgordon added bug 7.x Relates to a 7.x client version labels Feb 15, 2023
@stevejgordon stevejgordon added the 8.x Relates to 8.x client version label Apr 11, 2023
@stevejgordon
Copy link
Contributor

stevejgordon commented Apr 11, 2023

This also affects v8 because System.Text.Json also omits the decimal places for such dates. We need to review if it's possible to support forcing these or if we can provide workarounds that are nice than dropping to low-level code.

var stream = new MemoryStream();

var dateWithoutMilliseconds = new DateTime(2022, 6, 11, 13, 54, 20, 0);

client.RequestResponseSerializer.Serialize(new SearchRequestDescriptor<Person>().Query(q => q
   .Range(dr => dr.DateRange(d => d
   .Field("test_field")
   .Gt(dateWithoutMilliseconds))
)), stream);

stream.Position = 0;
var reader = new StreamReader(stream);
var result = reader.ReadToEnd(); // {"query":{"range":{"test_field":{"gt":"2022-06-11T13:54:20"}}}}

@YohDeadfall
Copy link
Contributor

YohDeadfall commented May 24, 2023

Well, my suggestion here is to improve serialization DateTime by using a custom converter first which isn't a problem, but it might be worth introducing an Elastic specific type later where only a part of the date time can be set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
7.x Relates to a 7.x client version 8.x Relates to 8.x client version Category: Bug
Projects
None yet
Development

No branches or pull requests

4 participants