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

Floating-point types are not always serialized with a decimal point #7051

Closed
Alkaersig opened this issue Nov 28, 2022 · 5 comments · Fixed by #7467
Closed

Floating-point types are not always serialized with a decimal point #7051

Alkaersig opened this issue Nov 28, 2022 · 5 comments · Fixed by #7467
Labels
8.x Relates to 8.x client version
Milestone

Comments

@Alkaersig
Copy link

Alkaersig commented Nov 28, 2022

Elastic.Clients.Elasticsearch version:
8.0.2

Elasticsearch version:
8.5.0

.NET runtime version:
6.0

Operating system version:
Windows 11

Description of the problem including expected versus actual behavior:
Ran into another problem while migrating one of our smaller projects to v8. Trying to index a range of documents i got an error that [float] could not be converted to [long]. Inspecting the request, it turns out that whenever a floating-point value does not have a decimal part, it is serialized as an int (without a decimal point/radix).

I guess this problem was introduced by the new Serializer in the v8 client. This was fixed some years ago back in v7:
#3657

Steps to reproduce:
1: Create a simple document

public class FloatDocument
{
  public decimal Amount { get; set; }
}

2: Insert a range of documents with a mix of integers and decimal numbers:

var data = new List<FloatDocument>();
data.Add(new FloatDocument() { Amount = 11 });
data.Add(new FloatDocument() { Amount = 1.15M });
data.Add(new FloatDocument() { Amount = 3.76M });

await esClient.IndexManyAsync<FloatDocument>(data, "index");

3: Returns an error: mapper [amount] cannot be changed from type [float] to [long]" since it serializes to:

[{
  Amount: 11
},
{
  Amount: 1.15
},
{
  Amount: 3.76
}]

Expected behavior
Floating-point types should always be serialized as floating-point types (serialized with a decimal point).

@Alkaersig Alkaersig added the 8.x Relates to 8.x client version label Nov 28, 2022
@stevejgordon
Copy link
Contributor

Thanks for raising this, @Alkaersig. I certainly see the problem this can introduce. We'll need to consider introducing custom converters to work around the choices made in System.Text.Json for such numbers. A workaround, for now, is to explicitly map fields in advance for the expected number type.

@Alkaersig
Copy link
Author

Hi @stevejgordon

Thanks for coming back on this. I already attempted the workaround, mapping our decimals to float using a template that creates the mapping when the index is created, but when trying to index a decimal that has no decimal value (like 10 (10.0)) , we still get the error (a [long] cannot be mapped to a [float]).

So i guess there is more to this, since i would expect it to map just fine to a float, even though ES parses it as a long (well at least if the long-value is not bigger than the max value of a float).

@stevejgordon
Copy link
Contributor

Oh, that's interesting. I thought I'd previously tested that without an issue. I'll take a look, and we'll find a solution.

@Alkaersig
Copy link
Author

Alkaersig commented Dec 20, 2022

Hi @stevejgordon

Just a little Christmas update.

We actually got it working now after updating from RC2 to 8.0.2 and mapping the field to float manually using index_templates. So the workaround do work after all.

So the issue only arises using auto-field-mapping when a long-value is encountered before a float (thus the field is mapped as long) which obviously does not support float-values :)

I updated the bug-report to reflect this (the example now has a long value in the first object while the following objects holds a float-value).

@stevejgordon
Copy link
Contributor

Thanks for the update, @Alkaersig. That's good to know. It would also be possible to define your own custom System.Text.Json converter to assign to your properties to control their serialization.

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

Successfully merging a pull request may close this issue.

2 participants