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

5.0 RC4 - Scalar mappings are storing Decimal as double #2463

Closed
niemyjski opened this issue Dec 6, 2016 · 3 comments
Closed

5.0 RC4 - Scalar mappings are storing Decimal as double #2463

niemyjski opened this issue Dec 6, 2016 · 3 comments

Comments

@niemyjski
Copy link
Contributor

niemyjski commented Dec 6, 2016

I noticed an issue where you can't round trip Decimal.MaxValue as the mapping type is double. This occurs due to an overflow (Value was either too large or too small for a Decimal.). It might be good to have unit tests for all the c# numeric types and do max value of each type and then call AutoMap / Scalar and then attempt to round trip them. As precision will be lost or gained and it should be handled :).

The following snippet shows off what's happening in the simplest of terms (http://stackoverflow.com/questions/21337241/convert-from-decimal-to-single-and-back-why-does-it-throw-an-overflow-exception).

double dbl = (double)Decimal.MaxValue;
decimal dec = (decimal)dbl;

I think this has to due with two things that should be noted. The types supported by elastic don't always match up to .NET Types and serialization.

@russcam
Copy link
Contributor

russcam commented Dec 8, 2016

related elastic/elasticsearch#17006

russcam added a commit that referenced this issue Dec 14, 2016
Bump AsciiDoctNet to support additional asciidoc syntax
Closes #2463
@russcam
Copy link
Contributor

russcam commented Dec 14, 2016

We discussed earlier this week about what we can or should do regarding this.

We decided that we would document the usage of decimal POCO property types in the automapping documentation (I think a separate section on types would be better in the future; I'll add an item to #2248) and how these map to Elasticsearch's double type. Attempting to handle an overflow exception in the client seems disingenuous, even if it originates from a Decimal.MinValue or Decimal.MaxValue conversion to double.

Hope that makes sense.

@niemyjski
Copy link
Contributor Author

niemyjski commented Dec 15, 2016

Yeah, I'm not sure, but I can tell you I would't read the xml docs for to see the conversion warning and I don't use auto mapping. I'd just expect it to work. I totally understand why it doesn't, but will other developers.

Mpdreamz pushed a commit that referenced this issue Dec 16, 2016
…es (#2495)

Bump AsciiDoctNet to support additional asciidoc syntax
Closes #2463
Mpdreamz pushed a commit that referenced this issue Dec 16, 2016
…es (#2495)

Bump AsciiDoctNet to support additional asciidoc syntax
Closes #2463
Conflicts:
	src/Tests/ClientConcepts/HighLevel/Mapping/AutoMap.doc.cs
awelburn pushed a commit to Artesian/elasticsearch-net that referenced this issue Nov 6, 2017
…es (elastic#2495)

Bump AsciiDoctNet to support additional asciidoc syntax
Closes elastic#2463
Conflicts:
	src/Tests/ClientConcepts/HighLevel/Mapping/AutoMap.doc.cs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants