Repository: thrift Updated Branches: refs/heads/master 9b9366145 -> 228b328f7
THRIFT-3376 C# and Python JSON protocol double values lose precision Client: C#, Python, C++, Ruby Patch: Nobuaki Sukegawa <[email protected]> This closes #643 Project: http://git-wip-us.apache.org/repos/asf/thrift/repo Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/228b328f Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/228b328f Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/228b328f Branch: refs/heads/master Commit: 228b328f7bffe4d03bc22027d5c6af58251dc3d0 Parents: 9b93661 Author: Nobuaki Sukegawa <[email protected]> Authored: Sat Oct 10 03:11:49 2015 +0900 Committer: Jens Geyer <[email protected]> Committed: Wed Oct 14 00:40:23 2015 +0200 ---------------------------------------------------------------------- lib/csharp/src/Protocol/TJSONProtocol.cs | 2 +- lib/csharp/test/ThriftTest/TestClient.cs | 8 ++++ lib/py/src/protocol/TJSONProtocol.py | 7 +-- test/cpp/src/TestClient.cpp | 69 +++++++++++++++++++++++++-- test/py/TestClient.py | 1 + test/rb/integration/TestClient.rb | 2 +- 6 files changed, 79 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/thrift/blob/228b328f/lib/csharp/src/Protocol/TJSONProtocol.cs ---------------------------------------------------------------------- diff --git a/lib/csharp/src/Protocol/TJSONProtocol.cs b/lib/csharp/src/Protocol/TJSONProtocol.cs index 4081d6e..88d554e 100644 --- a/lib/csharp/src/Protocol/TJSONProtocol.cs +++ b/lib/csharp/src/Protocol/TJSONProtocol.cs @@ -505,7 +505,7 @@ namespace Thrift.Protocol private void WriteJSONDouble(double num) { context.Write(); - String str = num.ToString(CultureInfo.InvariantCulture); + String str = num.ToString("R", CultureInfo.InvariantCulture); bool special = false; switch (str[0]) http://git-wip-us.apache.org/repos/asf/thrift/blob/228b328f/lib/csharp/test/ThriftTest/TestClient.cs ---------------------------------------------------------------------- diff --git a/lib/csharp/test/ThriftTest/TestClient.cs b/lib/csharp/test/ThriftTest/TestClient.cs index c09a801..f0297d0 100644 --- a/lib/csharp/test/ThriftTest/TestClient.cs +++ b/lib/csharp/test/ThriftTest/TestClient.cs @@ -358,6 +358,14 @@ namespace Test Console.WriteLine("*** FAILED ***"); returnCode |= ErrorBaseTypes; } + Console.Write("testDouble(-0.000341012439638598279)"); + dub = client.testDouble(-0.000341012439638598279); + Console.WriteLine(" = " + dub); + if (-0.000341012439638598279 != dub) + { + Console.WriteLine("*** FAILED ***"); + returnCode |= ErrorBaseTypes; + } byte[] binOut = PrepareTestData(true); Console.Write("testBinary(" + BytesToHex(binOut) + ")"); http://git-wip-us.apache.org/repos/asf/thrift/blob/228b328f/lib/py/src/protocol/TJSONProtocol.py ---------------------------------------------------------------------- diff --git a/lib/py/src/protocol/TJSONProtocol.py b/lib/py/src/protocol/TJSONProtocol.py index 9e25f67..7807a6c 100644 --- a/lib/py/src/protocol/TJSONProtocol.py +++ b/lib/py/src/protocol/TJSONProtocol.py @@ -176,9 +176,9 @@ class TJSONProtocolBase(TProtocolBase): self.context.write() self.trans.write(json.dumps(string, ensure_ascii=False)) - def writeJSONNumber(self, number): + def writeJSONNumber(self, number, formatter='{}'): self.context.write() - jsNumber = str(number) + jsNumber = formatter.format(number) if self.context.escapeNum(): jsNumber = "%s%s%s" % (QUOTE, jsNumber, QUOTE) self.trans.write(jsNumber) @@ -467,7 +467,8 @@ class TJSONProtocol(TJSONProtocolBase): self.writeJSONNumber(i64) def writeDouble(self, dbl): - self.writeJSONNumber(dbl) + # 17 significant digits should be just enough for any double precision value. + self.writeJSONNumber(dbl, '{:.17g}') def writeString(self, string): self.writeJSONString(string) http://git-wip-us.apache.org/repos/asf/thrift/blob/228b328f/test/cpp/src/TestClient.cpp ---------------------------------------------------------------------- diff --git a/test/cpp/src/TestClient.cpp b/test/cpp/src/TestClient.cpp index 1ab4d21..2736ee8 100644 --- a/test/cpp/src/TestClient.cpp +++ b/test/cpp/src/TestClient.cpp @@ -106,6 +106,29 @@ static void testVoid_clientReturn(const char* host, } } +// Workaround for absense of C++11 "auto" keyword. +template <typename T> +bool print_eq(T expected, T actual) { + cout << "(" << actual << ")" << endl; + if (expected != actual) { + cout << "*** FAILED ***" << endl << "Expected: " << expected << " but got: " << actual << endl; + return false; + } + return true; +} + +#define BASETYPE_IDENTITY_TEST(func, value) \ + cout << #func "(" << value << ") = "; \ + try { \ + if (!print_eq(value, testClient.func(value))) \ + return_code |= ERR_BASETYPES; \ + } catch (TTransportException&) { \ + throw; \ + } catch (exception & ex) { \ + cout << "*** FAILED ***" << endl << ex.what() << endl; \ + return_code |= ERR_BASETYPES; \ + } + int main(int argc, char** argv) { int ERR_BASETYPES = 1; int ERR_STRUCTS = 2; @@ -388,11 +411,47 @@ int main(int argc, char** argv) { /** * DOUBLE TEST */ - printf("testDouble(-5.2098523)"); - double dub = testClient.testDouble(-5.2098523); - printf(" = %f\n", dub); - if ((dub - (-5.2098523)) > 0.001) { - printf("*** FAILED ***\n"); + // Comparing double values with plain equality because Thrift handles full precision of double + BASETYPE_IDENTITY_TEST(testDouble, 0.0); + BASETYPE_IDENTITY_TEST(testDouble, -1.0); + BASETYPE_IDENTITY_TEST(testDouble, -5.2098523); + BASETYPE_IDENTITY_TEST(testDouble, -0.000341012439638598279); + BASETYPE_IDENTITY_TEST(testDouble, pow(2, 32)); + BASETYPE_IDENTITY_TEST(testDouble, pow(2, 32) + 1); + BASETYPE_IDENTITY_TEST(testDouble, pow(2, 53) - 1); + BASETYPE_IDENTITY_TEST(testDouble, -pow(2, 32)); + BASETYPE_IDENTITY_TEST(testDouble, -pow(2, 32) - 1); + BASETYPE_IDENTITY_TEST(testDouble, -pow(2, 53) + 1); + + try { + double expected = pow(10, 307); + cout << "testDouble(" << expected << ") = "; + double actual = testClient.testDouble(expected); + cout << "(" << actual << ")" << endl; + if (expected - actual > pow(10, 292)) { + cout << "*** FAILED ***" << endl + << "Expected: " << expected << " but got: " << actual << endl; + } + } catch (TTransportException&) { + throw; + } catch (exception& ex) { + cout << "*** FAILED ***" << endl << ex.what() << endl; + return_code |= ERR_BASETYPES; + } + + try { + double expected = pow(10, -292); + cout << "testDouble(" << expected << ") = "; + double actual = testClient.testDouble(expected); + cout << "(" << actual << ")" << endl; + if (expected - actual > pow(10, -307)) { + cout << "*** FAILED ***" << endl + << "Expected: " << expected << " but got: " << actual << endl; + } + } catch (TTransportException&) { + throw; + } catch (exception& ex) { + cout << "*** FAILED ***" << endl << ex.what() << endl; return_code |= ERR_BASETYPES; } http://git-wip-us.apache.org/repos/asf/thrift/blob/228b328f/test/py/TestClient.py ---------------------------------------------------------------------- diff --git a/test/py/TestClient.py b/test/py/TestClient.py index 7e3daf2..3a74353 100755 --- a/test/py/TestClient.py +++ b/test/py/TestClient.py @@ -158,6 +158,7 @@ class AbstractTest(unittest.TestCase): self.assertEqual(self.client.testDouble(-5.235098235), -5.235098235) self.assertEqual(self.client.testDouble(0), 0) self.assertEqual(self.client.testDouble(-1), -1) + self.assertEqual(self.client.testDouble(-0.000341012439638598279), -0.000341012439638598279) def testBinary(self): if isinstance(self, JSONTest): http://git-wip-us.apache.org/repos/asf/thrift/blob/228b328f/test/rb/integration/TestClient.rb ---------------------------------------------------------------------- diff --git a/test/rb/integration/TestClient.rb b/test/rb/integration/TestClient.rb index 8fd6336..6aec596 100755 --- a/test/rb/integration/TestClient.rb +++ b/test/rb/integration/TestClient.rb @@ -130,7 +130,7 @@ class SimpleClientTest < Test::Unit::TestCase def test_double p 'test_double' - val = 3.14 + val = 3.14159265358979323846 assert_equal(@client.testDouble(val), val) assert_equal(@client.testDouble(-val), -val) assert_kind_of(Float, @client.testDouble(val))
