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

Conversion of double to string can result in a loss of precision #408

Closed
btasker opened this issue Nov 9, 2022 · 8 comments · Fixed by #434
Closed

Conversion of double to string can result in a loss of precision #408

btasker opened this issue Nov 9, 2022 · 8 comments · Fixed by #434
Milestone

Comments

@btasker
Copy link

btasker commented Nov 9, 2022

When the client starts building LP, it converts double and float values to a string:

                if (value is double || value is float)
                {
                    sb.Append(((IConvertible)value).ToString(CultureInfo.InvariantCulture));
                }

However, CultureInfo.InvariantCulture may not preserve full precision.

Whereas, if that's replaced with "R" the precision will be maintained:

For example

using System;
using System.Globalization;
					
public class Program
{
	public static void Main()
	{
		double a = (double) 459.29587181322927;
		
		string b = a.ToString(CultureInfo.InvariantCulture);
		Console.WriteLine(b);
		
		string c = a.ToString("R");
		Console.WriteLine(c);
		
	}
}

Generates

459.295871813229
459.29587181322927

However, this isn't a full solution, because the use of R may also add unexpected precision. If we redefine a to be 45.29587181323112 in the example above, then we get 45.295871813231123 because of the way that C# handles conversion.

Steps to reproduce:
List the minimal actions needed to reproduce the behavior.

  1. Use the client
  2. Define a double with >15 significant numbers
  3. Write into InfluxDB

Expected behavior:
The full precision should be sent

Actual behavior:
The value will have lost precision

Specifications:

  • Client Version:
  • InfluxDB Version: 1.x/2.x
  • Platform: 4.7.0
@bednar
Copy link
Contributor

bednar commented Nov 11, 2022

Hi @btasker,

It looks like that the generated output depends on the platform. On my laptop the generated output is:

459.29587181322927
459.29587181322927

My platform:

/usr/local/share/dotnet/dotnet --info   
.NET SDK (reflecting any global.json):
 Version:   6.0.101
 Commit:    ef49f6213a

Runtime Environment:
 OS Name:     Mac OS X
 OS Version:  12.6
 OS Platform: Darwin
 RID:         osx.12-arm64
 Base Path:   /usr/local/share/dotnet/sdk/6.0.101/

Host (useful for support):
  Version: 6.0.1
  Commit:  3a25a7f1cc

.NET SDKs installed:
  6.0.101 [/usr/local/share/dotnet/sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 6.0.1 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 6.0.1 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]

To install additional .NET runtimes or SDKs:
  https://aka.ms/dotnet-download

What platform does the customer use?

According to the dotnet documentation the solution can be a G17 formatter if the customer uses old dotnet version:
image

Regards

@btasker
Copy link
Author

btasker commented Nov 11, 2022

Hey @bednar

Agreed - it looks like I can't repro in Docker on Linux either

root@29fb7ae75cda:/project# dotnet run
459.29587181322927
459.29587181322927

As MS have the tags, I've tested with SDK 5.0 and can't repro there either.

I believe the customer is on Windows - I'll check which version (and SDK etc).

@btasker
Copy link
Author

btasker commented Nov 11, 2022

My earlier Repro was done using this - https://dotnetfiddle.net/srx9kM

Looking a bit closer at that, by default it's using .NET 4.7.2. If I switch it to using .NET 6 the results are as expected - so this probably does only affect older versions

@bednar
Copy link
Contributor

bednar commented Nov 14, 2022

I tested on .NET 6 and Windows 11 and everything worked correctly.

@btasker
Copy link
Author

btasker commented Nov 14, 2022

Thanks @bednar

So, looks like this only affects EOL'd .NET versions (5.0 went EOL in May, 4 in April)

@bednar
Copy link
Contributor

bednar commented Nov 14, 2022

Do we have a confirmed version of ".NET" from the customer?

@btasker
Copy link
Author

btasker commented Nov 14, 2022

Not currently, I asked last week but haven't heard back. Will chase up later today.

@btasker
Copy link
Author

btasker commented Nov 28, 2022

@bednar The customer is running 4.6.1

However, they've noted that the Net 7 docs refer to this http://msdn.microsoft.com/en-us/library/kfsatb94.aspx

“By default, the return value only contains 15 digits of precision although a maximum of 17 digits is maintained internally. If the value of this instance has greater than 15 digits, ToString returns PositiveInfinitySymbol or NegativeInfinitySymbol instead of the expected number. If you require more precision, specify format with the "G17" format specification, which always returns 17 digits of precision, or "R", which returns 15 digits if the number can be represented with that precision or 17 digits if the number can only be represented with maximum precision.”

powersj added a commit that referenced this issue Dec 5, 2022
powersj added a commit that referenced this issue Dec 5, 2022
powersj added a commit that referenced this issue Dec 5, 2022
powersj added a commit that referenced this issue Dec 5, 2022
powersj added a commit that referenced this issue Dec 5, 2022
powersj added a commit that referenced this issue Dec 6, 2022
@bednar bednar added this to the 4.9.0 milestone Dec 6, 2022
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

Successfully merging a pull request may close this issue.

2 participants