From 086cdca90818e4114dc1ee3b2e6e7e5f2c41fe98 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Thu, 29 Jun 2023 15:27:44 +0300 Subject: [PATCH 1/2] msgpack: support decimals with negative scale Current decimal external type parser do not expect negative scale in decimal payload. Negative scale is a positive exponent. It seems that the only way to obtain positive exponent is to use E-notation since Tarantool library do not truncate trailing zeroes before decimal point: ``` tarantool> msgpack.encode(decimal.new('1e33')):hex() --- - c70301d0df1c ... tarantool> msgpack.encode(decimal.new('1000000000000000000000000000000000')):hex() --- - c713010001000000000000000000000000000000000c ... ``` There are two different bugs in current implementation: - we support only `positive fixint` scale and do not expect `int 8` [2], - we do not expect negative scale so positive exponent will be ignored. This patch fixes both of them. See also [3]. 1. https://github.com/tarantool/tarantool/blob/ba749e820bf0638aa3f79f266848590f9713c1cf/src/lib/core/decimal.c#L432-L450 2. https://github.com/msgpack/msgpack/blob/master/spec.md 3. https://github.com/tarantool/go-tarantool/pull/314 --- CHANGELOG.md | 3 +++ tarantool/msgpack_ext/decimal.py | 19 +++++++++++++++--- test/suites/test_decimal.py | 33 ++++++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f1e78b8..68408037 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Allow to require specific server protocol version and features (#267). +### Fixed +- Parsing of E-notation Tarantool decimals with positive exponent (PR #298). + ## 1.0.0 - 2023-04-17 ### Changed diff --git a/tarantool/msgpack_ext/decimal.py b/tarantool/msgpack_ext/decimal.py index 545ee07d..f1654784 100644 --- a/tarantool/msgpack_ext/decimal.py +++ b/tarantool/msgpack_ext/decimal.py @@ -56,6 +56,8 @@ from decimal import Decimal +import msgpack + from tarantool.error import MsgpackError, MsgpackWarning, warn EXT_ID = 1 @@ -353,7 +355,12 @@ def decode(data, _): :raise: :exc:`~tarantool.error.MsgpackError` """ - scale = data[0] + # A decimal starts with mp_int or mp_uint followed by raw bytes. + unpacker = msgpack.Unpacker() + unpacker.feed(data) + + scale = unpacker.unpack() + scale_size = unpacker.tell() sign = get_str_sign(data[-1] & 0x0f) @@ -362,7 +369,7 @@ def decode(data, _): add_str_digit(data[-1] >> 4, digits_reverted, scale) - for i in range(len(data) - 2, 0, -1): + for i in range(len(data) - 2, scale_size - 1, -1): add_str_digit(data[i] & 0x0f, digits_reverted, scale) add_str_digit(data[i] >> 4, digits_reverted, scale) @@ -372,6 +379,12 @@ def decode(data, _): digits_reverted.append(sign) - str_repr = ''.join(digits_reverted[::-1]) + digits = digits_reverted[::-1] + + # Add trailing zeroes in case of a negative scale + for i in range(0, -1 * scale): + add_str_digit(0, digits, scale) + + str_repr = ''.join(digits) return Decimal(str_repr) diff --git a/test/suites/test_decimal.py b/test/suites/test_decimal.py index c817ad08..b880eaa3 100644 --- a/test/suites/test_decimal.py +++ b/test/suites/test_decimal.py @@ -226,6 +226,39 @@ def setUp(self): b'\x09\x87\x65\x43\x21\x98\x76\x54\x32\x1d'), 'tarantool': "decimal.new('-1234567891234567890.0987654321987654321')", }, + 'decimal_exponent_1': { + 'python': decimal.Decimal('1e33'), + 'msgpack': (b'\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00' + b'\x00\x00\x00\x00\x00\x00\x00\x0c'), + 'tarantool': "decimal.new('1e33')", + }, + 'decimal_exponent_2': { + 'python': decimal.Decimal('1.2345e33'), + 'msgpack': (b'\x00\x01\x23\x45\x00\x00\x00\x00\x00\x00\x00' + b'\x00\x00\x00\x00\x00\x00\x00\x0c'), + 'tarantool': "decimal.new('1.2345e33')", + }, + 'decimal_exponent_3': { + 'python': decimal.Decimal('1.2345e2'), + 'msgpack': (b'\x02\x12\x34\x5c'), + 'tarantool': "decimal.new('1.2345e2')", + }, + 'decimal_exponent_4': { + 'python': decimal.Decimal('1.2345e4'), + 'msgpack': (b'\x00\x12\x34\x5c'), + 'tarantool': "decimal.new('1.2345e4')", + }, + 'decimal_exponent_5': { + 'python': decimal.Decimal('-1e33'), + 'msgpack': (b'\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00' + b'\x00\x00\x00\x00\x00\x00\x00\x0d'), + 'tarantool': "decimal.new('-1e33')", + }, + 'decimal_exponent_6': { + 'python': decimal.Decimal('1e-33'), + 'msgpack': (b'\x21\x1c'), + 'tarantool': "decimal.new('1e-33')", + }, } def test_msgpack_decode(self): From 78a6123ff895e486c2cbdb4b8dd7022af5019c85 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Thu, 29 Jun 2023 16:34:08 +0300 Subject: [PATCH 2/2] deps: drop msgpack-python support msgpack-python package was deprecated in 2018 in favor of msgpack package. After previous patch in the patchset, we require `Unpacker` `tell` handle and msgpack-python 0.4.0 do not yet have it. It seems to be no reason to support the package which is not supported for the five years already, so we better drop it. --- .github/workflows/testing.yml | 3 --- CHANGELOG.md | 3 +++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml index 25aa6b30..199744f2 100644 --- a/.github/workflows/testing.yml +++ b/.github/workflows/testing.yml @@ -43,9 +43,6 @@ jobs: # "This page is taking too long to load." error. Thus we use # pairwise testing. include: - - tarantool: '2.11' - python: '3.11' - msgpack-deps: 'msgpack-python==0.4.0' - tarantool: '2.11' python: '3.11' msgpack-deps: 'msgpack==0.5.0' diff --git a/CHANGELOG.md b/CHANGELOG.md index 68408037..255846bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Allow to require specific server protocol version and features (#267). +### Changed +- Drop `msgpack-python` support. Use `msgpack` instead. + ### Fixed - Parsing of E-notation Tarantool decimals with positive exponent (PR #298).