fix(datatype): Fix data ingestion to properly handle NUMERIC data types when using the binary transfer protocol to prevent incorrect results. issue #207

Author: Brooke-white

redshift_connector/utils/type_utils.py

@@ -245,11 +245,8 @@ def intervald2s_send_integer(v: IntervalDayToSecond) -> bytes:
 def numeric_in_binary(data: bytes, offset: int, length: int, scale: int) -> Decimal:
     raw_value: int
 
-    if length == 8:
-        raw_value = q_unpack(data, offset)[0]
-    elif length == 16:
-        temp: typing.Tuple[int, int] = qq_unpack(data, offset)
-        raw_value = (temp[0] << 64) | temp[1]
+    if length == 8 or length == 16:
+        raw_value = int.from_bytes(data[offset : offset + length], byteorder="big", signed=True)
     else:
         raise Exception("Malformed column value of type numeric received")
 
@@ -259,11 +256,8 @@ def numeric_in_binary(data: bytes, offset: int, length: int, scale: int) -> Deci
 def numeric_to_float_binary(data: bytes, offset: int, length: int, scale: int) -> float:
     raw_value: int
 
-    if length == 8:
-        raw_value = q_unpack(data, offset)[0]
-    elif length == 16:
-        temp: typing.Tuple[int, int] = qq_unpack(data, offset)
-        raw_value = (temp[0] << 64) | temp[1]
+    if length == 8 or length == 16:
+        raw_value = int.from_bytes(data[offset : offset + length], byteorder="big", signed=True)
     else:
         raise Exception("Malformed column value of type numeric received")
 

test/integration/datatype/test_datatypes.py

@@ -242,8 +242,13 @@ def test_abstime(db_kwargs, _input, client_protocol):
 
 numeric_vals: typing.List[typing.Tuple[str, float]] = [
     ("to_number('12,454.8-', 'S99G999D9')", -12454.8),
+    ("to_number('12,454.8', 'S99G999D9')", 12454.8),
     ("to_number('8.1-', '9D9S')", -8.1),
+    ("to_number('8.1', '9D9S')", 8.1),
     ("to_number('897.6', '999D9S')", 897.6),
+    ("to_number('897.6-', '999D9S')", -897.6),
+    ("to_number('$ 2,012,454.88', 'L 9,999,999.99');", 2012454.88),
+    ("to_number('$ -2,012,454.88', 'L 9,999,999.99');", -2012454.88),
 ]
 
 
@@ -263,3 +268,47 @@ def test_numeric_to_float(db_kwargs, _input, client_protocol):
                 rel_tol=1e-05,
                 abs_tol=1e-08,
             )
+
+
+numeric_precision_scale_vals: typing.List[typing.Tuple[str, float]] = [
+    ("-135430.11999999999500::numeric(36,14)", -135430.11999999999500),
+    ("g", 135430.11999999999500),
+    ("-35430.11999999999500::numeric(36,14)", -35430.11999999999500),
+    ("35430.11999999999500::numeric(36,14)", 35430.11999999999500),
+    ("-7872432525245.4577::numeric(36,14)", -7872432525245.4577),
+    ("7872432525245.4577::numeric(36,14)", 7872432525245.4577),
+    ("-252252::numeric(36,14)", -252252),
+    ("252252::numeric(36,14)", 252252),
+    ("-135430.11999999999500::numeric(30,19)", -135430.11999999999500),
+    ("135430.11999999999500::numeric(36,19)", 135430.11999999999500),
+    ("-35430.11999999999500::numeric(36,19)", -35430.11999999999500),
+    ("35430.11999999999500::numeric(36,19)", 35430.11999999999500),
+    ("-252252::numeric(36,11)", -252252),
+    ("252252::numeric(36,11)", 252252),
+    ("-135430.11999999999500::numeric(36,11)", -135430.11999999999500),
+    ("135430.11999999999500::numeric(36,11)", 135430.11999999999500),
+    ("-35430.11999999999500::numeric(36,11)", -35430.11999999999500),
+    ("35430.11999999999500::numeric(36,11)", 35430.11999999999500),
+    ("-7872432525245.4577::numeric(36,11)", -7872432525245.4577),
+    ("7872432525245.4577::numeric(36,11)", 7872432525245.4577),
+    ("-252252::numeric(36,11)", -252252),
+    ("252252::numeric(36,11)", 252252),
+]
+
+
+@pytest.mark.parametrize("client_protocol", ClientProtocolVersion.list())
+@pytest.mark.parametrize("_input", numeric_precision_scale_vals)
+def test_numeric_precision(db_kwargs, client_protocol, _input):
+    insert_val, exp_val = _input
+    db_kwargs["numeric_to_float"] = True
+    with redshift_connector.connect(**db_kwargs) as conn:
+        with conn.cursor() as cursor:
+            cursor.execute("select {}".format(insert_val))
+            res = cursor.fetchone()
+            assert isinstance(res[0], float)
+            assert isclose(
+                typing.cast(float, res[0]),
+                typing.cast(float, exp_val),
+                rel_tol=1e-05,
+                abs_tol=1e-08,
+            )

test/unit/datatype/test_data_in.py

@@ -25,6 +25,7 @@ class Datatypes(Enum):
     float8: typing.Callable = type_utils.float8_recv
     timestamp: typing.Callable = type_utils.timestamp_recv_integer
     numeric_binary: typing.Callable = type_utils.numeric_in_binary
+    numeric_to_float_binary: typing.Callable = type_utils.numeric_to_float_binary
     numeric: typing.Callable = type_utils.numeric_in
     timetz_binary: typing.Callable = type_utils.timetz_recv_binary
     time_binary: typing.Callable = type_utils.time_recv_binary
@@ -117,6 +118,73 @@ class Datatypes(Enum):
             datetime(year=2008, month=6, day=1, hour=9, minute=59, second=59),
         )
     ],
+    Datatypes.numeric_to_float_binary: [
+        # 8
+        (
+            b"\x00\x01\x00\x00\x00\x08\x01\xb6\x9bK\xac\xd0_\x15",
+            6,
+            8,
+            0,
+            Decimal(123456789123456789),
+        ),
+        (
+            b"\x00\x01\x00\x00\x00\x08\x00\x00\x00\x00I\x96\x02\xd3",
+            6,
+            8,
+            5,
+            Decimal(12345.67891),
+        ),
+        (
+            b"\x00\x01\x00\x00\x00\x08\xff\xff\xff\xff\xb6i\xfd-",
+            6,
+            8,
+            5,
+            Decimal(-12345.67891),
+        ),
+        (
+            b"\x00\x01\x00\x00\x00\x08\x00\x00\x00\x00\x00\x0009",
+            6,
+            8,
+            8,
+            Decimal(0.00012345),
+        ),
+        (
+            b"\x00\x01\x00\x00\x00\x08\x00\x00\x01\x1fq\xfb\x088",
+            6,
+            8,
+            8,
+            Decimal(12345.67891),
+        ),
+        (
+            b"\x00\x01\x00\x00\x00\x08\x00\x18\x1e\xab\xfb\xfaj\x9e",
+            6,
+            8,
+            3,
+            Decimal(6789123456789.15),
+        ),
+        (
+            b'\x00\x01\x00\x00\x00\x08\x11"\x10\xf4\xc0#\xb6\xd4',
+            6,
+            8,
+            1,
+            Decimal(123456789123456789.2),
+        ),
+        # 16
+        (
+            b"\x00\x01\x00\x00\x00\x10\tI\xb0\xf7\x13\xe9\x18_~\x8f\x1a\x99\xa9\x9b\xb6\xdb",
+            6,
+            16,
+            0,
+            Decimal(12345678912345678991234567891234567899),
+        ),
+        (
+            b"\x00\x01\x00\x00\x00\x10\x00\x02`\xb0`\x05\x18\xdb<\xd5\x01\x15\xd9\x8ek\x8d",
+            6,
+            16,
+            26,
+            Decimal(123456789.12345679104328155517578125),
+        ),
+    ],
     Datatypes.numeric_binary: [
         # 8
         (
@@ -154,6 +222,20 @@ class Datatypes(Enum):
             8,
             Decimal(12345.67891),
         ),
+        (
+            b"\x00\x03\x00\x00\x00\x08\x00\x18\x1e\xab\xfb\xfaj\x9e\x00\x00\x00\x01a\x00\x00\x00\x08\x01\xb6\x9bK\xac\xd0_\x15",
+            6,
+            8,
+            3,
+            Decimal(6789123456789.15),
+        ),
+        (
+            b"\x00\x03\x00\x00\x00\x08\x00\x18\x1e\xab\xfb\xfaj\x9e\x00\x00\x00\x01a\x00\x00\x00\x08\x01\xb6\x9bK\xac\xd0_\x15",
+            23,
+            8,
+            0,
+            Decimal(123456789123456789.2),
+        ),
         # 16
         (
             b"\x00\x05\x00\x00\x00\x10\tI\xb0\xf7\x13\xe9\x18_~\x8f\x1a\x99\xa9\x9b\xb6\xdb\x00\x00\x00\x10\tI\xb0\xf7\x13\xe9\x18_~\x8f\x1a\x99\xa9\x9b\xb6\xdb\x00\x00\x00\x10\tI\xb0\xf7\x13\xe9\x18_~\x8f\x1a\x99\xa9\x9b\xb6\xdb\x00\x00\x00\x10\tI\xb0\xf7\x13\xe9\x18_~\x8f\x1a\x99\xa9\x9b\xb6\xdb\x00\x00\x00\x10\tI\xb0\xf7\x13\xe9\x18_~\x8f\x1a\x99\xa9\x9b\xb6\xdb",
@@ -376,7 +458,7 @@ def get_test_cases() -> typing.Generator:
 @pytest.mark.parametrize("_input", get_test_cases(), ids=[k.__name__ for k, v in get_test_cases()])
 def test_datatype_recv(_input):
     test_func, test_args = _input
-    if len(test_args) == 5:  # numeric_in_binary
+    if len(test_args) == 5:  # numeric_in_binary or numeric_to_float_binary
         _data, _offset, _length, scale, exp_result = test_args
         assert isclose(test_func(_data, _offset, _length, scale), exp_result, rel_tol=1e-6)
     else:
@@ -385,3 +467,28 @@ def test_datatype_recv(_input):
             assert isclose(test_func(_data, _offset, _length), exp_result, rel_tol=1e-6)
         else:
             assert test_func(_data, _offset, _length) == exp_result
+
+
+invalid_numeric_lengths: typing.List[int] = [-1, 0, 7, 9, 15, 17, 99]
+
+
+@pytest.mark.parametrize("length", invalid_numeric_lengths)
+def test_numeric_in_binary_raises_for_invalid_length(length):
+    with pytest.raises(Exception, match="Malformed column value of type numeric received"):
+        Datatypes.numeric_binary(
+            b"\x00\x02\x00\x00\x00\x0c-12345.67891\x00\x00\x00\x08\xff\xff\xfe\xe0\x8e\x04\xf7\xc8",
+            22,
+            length,  # invalid length
+            8,
+        )
+
+
+@pytest.mark.parametrize("length", invalid_numeric_lengths)
+def test_numeric_to_float_binary_raises_for_invalid_length(length):
+    with pytest.raises(Exception, match="Malformed column value of type numeric received"):
+        Datatypes.numeric_to_float_binary(
+            b"\x00\x02\x00\x00\x00\x0c-12345.67891\x00\x00\x00\x08\xff\xff\xfe\xe0\x8e\x04\xf7\xc8",
+            22,
+            length,  # invalid length
+            8,
+        )