You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by je...@apache.org on 2015/10/14 00:45:53 UTC
thrift git commit: THRIFT-3376 C# and Python JSON protocol double
values lose precision Client: C#, Python, C++,
Ruby Patch: Nobuaki Sukegawa
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 <ns...@gmail.com>
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 <ns...@gmail.com>
Authored: Sat Oct 10 03:11:49 2015 +0900
Committer: Jens Geyer <je...@apache.org>
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))