Tarantool development patches archive
 help / color / mirror / Atom feed
* [PATCH 0/5] Decimal indices
@ 2019-07-17 15:33 Serge Petrenko
  2019-07-17 15:33 ` [PATCH 1/5] decimal: fix decimal.round() when scale == 0 Serge Petrenko
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Serge Petrenko @ 2019-07-17 15:33 UTC (permalink / raw)
  To: vdavydov.dev; +Cc: tarantool-patches, Serge Petrenko

https://github.com/tarantool/tarantool/issues/4333
https://github.com/tarantool/tarantool/tree/sp/gh-4333-decimal-index

This patchset adds the ability to insert decimal values into spaces and to build
indices over decimal fields.

The first patch fixes a bug in decimal.round() found during working on the
patchset. This bug prevented correct hint calculations for some decimal values.

The second patch updates msgpack library with the ability to encode/decode
decimals as msgpack

The third patch propagates the msgpack changes to msgpack Lua module.
This allows to insert decimals into spaces, but only into unindexed fields.

The fourth patch updates the decNumber library with new ToUInt64 and ToInt64
functions and adds the corresponding wrappers to our decimal module. These
functions are needed for hint calculation.

The fifth patch adds a new field type, 'decimal', allows to store decimal values
in 'decimal', 'scalar' and 'number' fields, and adds all the code necessary for
hint calculations over decimal fields.

Serge Petrenko (5):
  decimal: fix decimal.round() when scale == 0
  decimal: allow to encode/decode decimals as MsgPack
  lua: allow to encode and decode decimals as msgpack
  decimal: add conversions to (u)int64_t
  decimal: allow to index decimals

 extra/exports                 |   4 +
 src/box/field_def.c           |  43 ++++--
 src/box/field_def.h           |  16 +-
 src/box/key_def.h             |   2 +-
 src/box/tuple_compare.cc      | 160 +++++++++++++++++++-
 src/box/tuple_format.c        |   2 +-
 src/lib/core/CMakeLists.txt   |   1 +
 src/lib/core/decimal.c        |  81 ++++++++--
 src/lib/core/decimal.h        |  20 +++
 src/lib/core/mp_decimal.c     |  72 +++++++++
 src/lib/core/mp_decimal.h     |  70 +++++++++
 src/lib/core/mp_user_types.h  |  39 +++++
 src/lib/core/mpstream.c       |  11 ++
 src/lib/core/mpstream.h       |   4 +
 src/lua/decimal.c             |  10 +-
 src/lua/decimal.h             |   5 +
 src/lua/msgpack.c             |  52 +++++--
 src/lua/msgpackffi.lua        |  33 +++++
 src/lua/utils.c               |  16 +-
 src/lua/utils.h               |   8 +-
 test/app/decimal.result       | 116 +++++++++------
 test/app/decimal.test.lua     |   6 +
 test/app/msgpack.result       |  41 +++++
 test/app/msgpack.test.lua     |  15 ++
 test/engine/decimal.result    | 226 ++++++++++++++++++++++++++++
 test/engine/decimal.test.lua  |  65 ++++++++
 test/unit/decimal.c           | 124 +++++++++++++++-
 test/unit/decimal.result      | 272 +++++++++++++++++++++++++++++++++-
 third_party/decNumber         |   2 +-
 third_party/lua-yaml/lyaml.cc |   6 +
 30 files changed, 1420 insertions(+), 102 deletions(-)
 create mode 100644 src/lib/core/mp_decimal.c
 create mode 100644 src/lib/core/mp_decimal.h
 create mode 100644 src/lib/core/mp_user_types.h
 create mode 100644 test/engine/decimal.result
 create mode 100644 test/engine/decimal.test.lua

-- 
2.20.1 (Apple Git-117)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/5] decimal: fix decimal.round() when scale == 0
  2019-07-17 15:33 [PATCH 0/5] Decimal indices Serge Petrenko
@ 2019-07-17 15:33 ` Serge Petrenko
  2019-07-24 12:10   ` Vladimir Davydov
  2019-07-24 23:02   ` [tarantool-patches] " Konstantin Osipov
  2019-07-17 15:33 ` [PATCH 2/5] decimal: allow to encode/decode decimals as MsgPack Serge Petrenko
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 16+ messages in thread
From: Serge Petrenko @ 2019-07-17 15:33 UTC (permalink / raw)
  To: vdavydov.dev; +Cc: tarantool-patches, Serge Petrenko

Fixes decimal.round() with zero scale, fixes an error with
decimal.round() when rounding leads to a number with the same
precision, for example, decimal.round(9.9, 0) -> 10.

Follow-up #692
---
 src/lib/core/decimal.c    |  8 ++++----
 test/app/decimal.result   | 18 ++++++++++++++++++
 test/app/decimal.test.lua |  6 ++++++
 3 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/src/lib/core/decimal.c b/src/lib/core/decimal.c
index 1b4f44362..1423ae418 100644
--- a/src/lib/core/decimal.c
+++ b/src/lib/core/decimal.c
@@ -172,14 +172,14 @@ decimal_round(decimal_t *dec, int scale)
 	if (scale < 0 || scale > DECIMAL_MAX_DIGITS)
 		return NULL;
 
-	if (scale > decimal_scale(dec))
+	if (scale >= decimal_scale(dec))
 		return dec;
 
-	int ndig = decimal_precision(dec) - decimal_scale(dec) + scale;
+	int ndig = MAX(decimal_precision(dec) - decimal_scale(dec) + scale, 1);
 	decContext context = {
 		ndig, /* Precision */
-		ndig - 1, /* emax */
-		-1, /* emin */
+		ndig, /* emax */
+		scale != 0 ? -1 : 0, /* emin */
 		DECIMAL_ROUNDING, /* rounding */
 		0, /* no traps */
 		0, /* zero status */
diff --git a/test/app/decimal.result b/test/app/decimal.result
index ecb189277..53ec73edc 100644
--- a/test/app/decimal.result
+++ b/test/app/decimal.result
@@ -458,3 +458,21 @@ a ^ 2.5
  | ---
  | - error: '[string "return a ^ 2.5 "]:1: decimal operation failed'
  | ...
+
+-- check correct rounding when scale = 0
+decimal.round(decimal.new(0.9), 0)
+ | ---
+ | - '1'
+ | ...
+decimal.round(decimal.new(9.9), 0)
+ | ---
+ | - '10'
+ | ...
+decimal.round(decimal.new(99.9), 0)
+ | ---
+ | - '100'
+ | ...
+decimal.round(decimal.new(99.4), 0)
+ | ---
+ | - '99'
+ | ...
diff --git a/test/app/decimal.test.lua b/test/app/decimal.test.lua
index 4aff0f2a4..20fd4d81a 100644
--- a/test/app/decimal.test.lua
+++ b/test/app/decimal.test.lua
@@ -128,3 +128,9 @@ a = decimal.new('-13')
 a ^ 2
 -- fractional powers are allowed only for positive numbers
 a ^ 2.5
+
+-- check correct rounding when scale = 0
+decimal.round(decimal.new(0.9), 0)
+decimal.round(decimal.new(9.9), 0)
+decimal.round(decimal.new(99.9), 0)
+decimal.round(decimal.new(99.4), 0)
-- 
2.20.1 (Apple Git-117)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 2/5] decimal: allow to encode/decode decimals as MsgPack
  2019-07-17 15:33 [PATCH 0/5] Decimal indices Serge Petrenko
  2019-07-17 15:33 ` [PATCH 1/5] decimal: fix decimal.round() when scale == 0 Serge Petrenko
@ 2019-07-17 15:33 ` Serge Petrenko
  2019-07-24 16:19   ` Vladimir Davydov
  2019-07-24 23:08   ` [tarantool-patches] " Konstantin Osipov
  2019-07-17 15:33 ` [PATCH 3/5] lua: allow to encode and decode decimals as msgpack Serge Petrenko
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 16+ messages in thread
From: Serge Petrenko @ 2019-07-17 15:33 UTC (permalink / raw)
  To: vdavydov.dev; +Cc: tarantool-patches, Serge Petrenko

This patch adds the methods necessary to encode and decode decimals to
MsgPack. MsgPack EXT type (MP_EXT) together with a new extension type
MP_DECIMAL is used as a record header.

The decimal MsgPack representation looks like this:
+--------+-------------------+------------+===============+
| MP_EXT | length (optional) | MP_DECIMAL | PackedDecimal |
+--------+-------------------+------------+===============+
The whole record may be encoded and decoded with
mp_encode_decimal() and mp_decode_decimal(). This is equivalent to
performing mp_encode_extl()/mp_decode_extl() on the first 3 fields and
decimal_pack/unpack() on the PackedDecimal field.

Part of #692
---
 src/lib/core/CMakeLists.txt  |   1 +
 src/lib/core/mp_decimal.c    |  72 +++++++++++++
 src/lib/core/mp_decimal.h    |  62 +++++++++++
 src/lib/core/mp_user_types.h |  38 +++++++
 test/unit/decimal.c          |  60 ++++++++++-
 test/unit/decimal.result     | 202 ++++++++++++++++++++++++++++++++++-
 6 files changed, 433 insertions(+), 2 deletions(-)
 create mode 100644 src/lib/core/mp_decimal.c
 create mode 100644 src/lib/core/mp_decimal.h
 create mode 100644 src/lib/core/mp_user_types.h

diff --git a/src/lib/core/CMakeLists.txt b/src/lib/core/CMakeLists.txt
index 66e430a25..e0a717aa3 100644
--- a/src/lib/core/CMakeLists.txt
+++ b/src/lib/core/CMakeLists.txt
@@ -27,6 +27,7 @@ set(core_sources
     mpstream.c
     port.c
     decimal.c
+    mp_decimal.c
 )
 
 if (TARGET_OS_NETBSD)
diff --git a/src/lib/core/mp_decimal.c b/src/lib/core/mp_decimal.c
new file mode 100644
index 000000000..1bcf316fa
--- /dev/null
+++ b/src/lib/core/mp_decimal.c
@@ -0,0 +1,72 @@
+/*
+ * Copyright 2019, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ *    copyright notice, this list of conditions and the
+ *    following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials
+ *    provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include "mp_decimal.h"
+#include "mp_user_types.h"
+#include "msgpuck.h"
+#include "decimal.h"
+
+uint32_t
+mp_sizeof_decimal(const decimal_t *dec)
+{
+	return mp_sizeof_ext(decimal_len(dec));
+}
+
+decimal_t *
+mp_decode_decimal(const char **data, decimal_t *dec)
+{
+	if (mp_typeof(**data) != MP_EXT)
+		return NULL;
+
+	int8_t type;
+	uint32_t len;
+	const char *const svp = *data;
+
+	len = mp_decode_extl(data, &type);
+
+	if (type != MP_DECIMAL || len == 0) {
+		*data = svp;
+		return NULL;
+	}
+	decimal_t *res = decimal_unpack(data,  len, dec);
+	if (!res)
+		*data = svp;
+	return res;
+}
+
+char *
+mp_encode_decimal(char *data, const decimal_t *dec)
+{
+	uint32_t len = decimal_len(dec);
+	data = mp_encode_extl(data, MP_DECIMAL, len);
+	data = decimal_pack(data, dec);
+	return data;
+}
diff --git a/src/lib/core/mp_decimal.h b/src/lib/core/mp_decimal.h
new file mode 100644
index 000000000..a991a5f16
--- /dev/null
+++ b/src/lib/core/mp_decimal.h
@@ -0,0 +1,62 @@
+#ifndef TARANTOOL_LIB_CORE_MP_DECIMAL_INCLUDED
+#define TARANTOOL_LIB_CORE_MP_DECIMAL_INCLUDED
+/*
+ * Copyright 2019, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ *    copyright notice, this list of conditions and the
+ *    following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials
+ *    provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include "decimal.h"
+#include <stdint.h>
+
+/**
+ * \brief Calculate exact buffer size needed to store a decimal
+ * pointed to by \a dec.
+ */
+uint32_t
+mp_sizeof_decimal(const decimal_t *dec);
+
+/**
+ * \brief Decode a decimal from MsgPack \a data
+ * \param data - buffer pointer
+ * \return the decoded decimal
+ * \post *data = *data + mp_sizeof_decimal(retval)
+ */
+decimal_t *
+mp_decode_decimal(const char **data, decimal_t *dec);
+
+/**
+ * \brief Encode a decimal pointed to by \a dec.
+ * \parad dec - decimal pointer
+ * \param data - a buffer
+ * \return \a data + mp_sizeof_decimal(\a dec)
+ */
+char *
+mp_encode_decimal(char *data, const decimal_t *dec);
+
+#endif
diff --git a/src/lib/core/mp_user_types.h b/src/lib/core/mp_user_types.h
new file mode 100644
index 000000000..9158b40d3
--- /dev/null
+++ b/src/lib/core/mp_user_types.h
@@ -0,0 +1,38 @@
+#ifndef TARANTOOL_LIB_CORE_MP_USER_TYPES_H_INCLUDED
+#define TARANTOOL_LIB_CORE_MP_USER_TYPES_H_INCLUDED
+/*
+ * Copyright 2019, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ *    copyright notice, this list of conditions and the
+ *    following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials
+ *    provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+enum mp_user_type {
+    MP_DECIMAL = 0
+};
+
+#endif
diff --git a/test/unit/decimal.c b/test/unit/decimal.c
index b587e1f14..dd64b95ed 100644
--- a/test/unit/decimal.c
+++ b/test/unit/decimal.c
@@ -1,5 +1,8 @@
 #include "unit.h"
 #include "decimal.h"
+#include "mp_decimal.h"
+#include "mp_user_types.h"
+#include "msgpuck.h"
 #include <limits.h>
 #include <string.h>
 #include <float.h> /* DBL_DIG */
@@ -71,6 +74,32 @@
 
 char buf[32];
 
+#define test_mpdec(str) ({\
+	decimal_t dec;\
+	decimal_from_string(&dec, str);\
+	uint32_t l1 = mp_sizeof_decimal(&dec);\
+	ok(l1 <= 25 && l1 >= 4, "mp_sizeof_decimal("str")");\
+	char *b1 = mp_encode_decimal(buf, &dec);\
+	is(b1, buf + l1, "mp_sizeof_decimal("str") == len(mp_encode_decimal("str"))");\
+	const char *b2 = buf;\
+	const char *b3 = buf;\
+	decimal_t d2;\
+	mp_next(&b3);\
+	is(b3, b1, "mp_next(mp_encode("str"))");\
+	mp_decode_decimal(&b2, &d2);\
+	is(b1, b2, "mp_decode(mp_encode("str") len");\
+	is(decimal_compare(&dec, &d2), 0, "mp_decode(mp_encode("str")) value");\
+	is(decimal_scale(&dec), decimal_scale(&d2), "mp_decode(mp_encode("str")) scale");\
+	is(strcmp(decimal_to_string(&d2), str), 0, "str(mp_decode(mp_encode("str"))) == "str);\
+	b2 = buf;\
+	int8_t type;\
+	uint32_t l2 = mp_decode_extl(&b2, &type);\
+	is(type, MP_DECIMAL, "mp_ext_type is MP_DECIMAL");\
+	is(&d2, decimal_unpack(&b2, l2, &d2), "decimal_unpack() after mp_decode_extl()");\
+	is(decimal_compare(&dec, &d2), 0, "decimal_unpack() after mp_decode_extl() value");\
+	is(b2, buf + l1, "decimal_unpack() after mp_decode_extl() len");\
+})
+
 #define test_decpack(str) ({\
 	decimal_t dec;\
 	decimal_from_string(&dec, str);\
@@ -125,10 +154,37 @@ test_pack_unpack(void)
 	return check_plan();
 }
 
+static int
+test_mp_decimal(void)
+{
+	plan(198);
+
+	test_mpdec("0");
+	test_mpdec("-0");
+	test_mpdec("1");
+	test_mpdec("-1");
+	test_mpdec("0.1");
+	test_mpdec("-0.1");
+	test_mpdec("2.718281828459045");
+	test_mpdec("-2.718281828459045");
+	test_mpdec("3.141592653589793");
+	test_mpdec("-3.141592653589793");
+	test_mpdec("1234567891234567890.0987654321987654321");
+	test_mpdec("-1234567891234567890.0987654321987654321");
+	test_mpdec("0.0000000000000000000000000000000000001");
+	test_mpdec("-0.0000000000000000000000000000000000001");
+	test_mpdec("0.00000000000000000000000000000000000001");
+	test_mpdec("-0.00000000000000000000000000000000000001");
+	test_mpdec("99999999999999999999999999999999999999");
+	test_mpdec("-99999999999999999999999999999999999999");
+
+	return check_plan();
+}
+
 int
 main(void)
 {
-	plan(279);
+	plan(280);
 
 	dectest(314, 271, uint64, uint64_t);
 	dectest(65535, 23456, uint64, uint64_t);
@@ -190,5 +246,7 @@ main(void)
 
 	test_pack_unpack();
 
+	test_mp_decimal();
+
 	return check_plan();
 }
diff --git a/test/unit/decimal.result b/test/unit/decimal.result
index 1c72cdfab..6d00996d3 100644
--- a/test/unit/decimal.result
+++ b/test/unit/decimal.result
@@ -1,4 +1,4 @@
-1..279
+1..280
 ok 1 - decimal(314)
 ok 2 - decimal(271)
 ok 3 - decimal(314) + decimal(271)
@@ -425,3 +425,203 @@ ok 278 - decimal_sqrt(-10) - error on wrong operands.
     ok 145 - unpack malformed decimal fails
     ok 146 - decode malformed decimal preserves buffer position
 ok 279 - subtests
+    1..198
+    ok 1 - mp_sizeof_decimal(0)
+    ok 2 - mp_sizeof_decimal(0) == len(mp_encode_decimal(0))
+    ok 3 - mp_next(mp_encode(0))
+    ok 4 - mp_decode(mp_encode(0) len
+    ok 5 - mp_decode(mp_encode(0)) value
+    ok 6 - mp_decode(mp_encode(0)) scale
+    ok 7 - str(mp_decode(mp_encode(0))) == 0
+    ok 8 - mp_ext_type is MP_DECIMAL
+    ok 9 - decimal_unpack() after mp_decode_extl()
+    ok 10 - decimal_unpack() after mp_decode_extl() value
+    ok 11 - decimal_unpack() after mp_decode_extl() len
+    ok 12 - mp_sizeof_decimal(-0)
+    ok 13 - mp_sizeof_decimal(-0) == len(mp_encode_decimal(-0))
+    ok 14 - mp_next(mp_encode(-0))
+    ok 15 - mp_decode(mp_encode(-0) len
+    ok 16 - mp_decode(mp_encode(-0)) value
+    ok 17 - mp_decode(mp_encode(-0)) scale
+    ok 18 - str(mp_decode(mp_encode(-0))) == -0
+    ok 19 - mp_ext_type is MP_DECIMAL
+    ok 20 - decimal_unpack() after mp_decode_extl()
+    ok 21 - decimal_unpack() after mp_decode_extl() value
+    ok 22 - decimal_unpack() after mp_decode_extl() len
+    ok 23 - mp_sizeof_decimal(1)
+    ok 24 - mp_sizeof_decimal(1) == len(mp_encode_decimal(1))
+    ok 25 - mp_next(mp_encode(1))
+    ok 26 - mp_decode(mp_encode(1) len
+    ok 27 - mp_decode(mp_encode(1)) value
+    ok 28 - mp_decode(mp_encode(1)) scale
+    ok 29 - str(mp_decode(mp_encode(1))) == 1
+    ok 30 - mp_ext_type is MP_DECIMAL
+    ok 31 - decimal_unpack() after mp_decode_extl()
+    ok 32 - decimal_unpack() after mp_decode_extl() value
+    ok 33 - decimal_unpack() after mp_decode_extl() len
+    ok 34 - mp_sizeof_decimal(-1)
+    ok 35 - mp_sizeof_decimal(-1) == len(mp_encode_decimal(-1))
+    ok 36 - mp_next(mp_encode(-1))
+    ok 37 - mp_decode(mp_encode(-1) len
+    ok 38 - mp_decode(mp_encode(-1)) value
+    ok 39 - mp_decode(mp_encode(-1)) scale
+    ok 40 - str(mp_decode(mp_encode(-1))) == -1
+    ok 41 - mp_ext_type is MP_DECIMAL
+    ok 42 - decimal_unpack() after mp_decode_extl()
+    ok 43 - decimal_unpack() after mp_decode_extl() value
+    ok 44 - decimal_unpack() after mp_decode_extl() len
+    ok 45 - mp_sizeof_decimal(0.1)
+    ok 46 - mp_sizeof_decimal(0.1) == len(mp_encode_decimal(0.1))
+    ok 47 - mp_next(mp_encode(0.1))
+    ok 48 - mp_decode(mp_encode(0.1) len
+    ok 49 - mp_decode(mp_encode(0.1)) value
+    ok 50 - mp_decode(mp_encode(0.1)) scale
+    ok 51 - str(mp_decode(mp_encode(0.1))) == 0.1
+    ok 52 - mp_ext_type is MP_DECIMAL
+    ok 53 - decimal_unpack() after mp_decode_extl()
+    ok 54 - decimal_unpack() after mp_decode_extl() value
+    ok 55 - decimal_unpack() after mp_decode_extl() len
+    ok 56 - mp_sizeof_decimal(-0.1)
+    ok 57 - mp_sizeof_decimal(-0.1) == len(mp_encode_decimal(-0.1))
+    ok 58 - mp_next(mp_encode(-0.1))
+    ok 59 - mp_decode(mp_encode(-0.1) len
+    ok 60 - mp_decode(mp_encode(-0.1)) value
+    ok 61 - mp_decode(mp_encode(-0.1)) scale
+    ok 62 - str(mp_decode(mp_encode(-0.1))) == -0.1
+    ok 63 - mp_ext_type is MP_DECIMAL
+    ok 64 - decimal_unpack() after mp_decode_extl()
+    ok 65 - decimal_unpack() after mp_decode_extl() value
+    ok 66 - decimal_unpack() after mp_decode_extl() len
+    ok 67 - mp_sizeof_decimal(2.718281828459045)
+    ok 68 - mp_sizeof_decimal(2.718281828459045) == len(mp_encode_decimal(2.718281828459045))
+    ok 69 - mp_next(mp_encode(2.718281828459045))
+    ok 70 - mp_decode(mp_encode(2.718281828459045) len
+    ok 71 - mp_decode(mp_encode(2.718281828459045)) value
+    ok 72 - mp_decode(mp_encode(2.718281828459045)) scale
+    ok 73 - str(mp_decode(mp_encode(2.718281828459045))) == 2.718281828459045
+    ok 74 - mp_ext_type is MP_DECIMAL
+    ok 75 - decimal_unpack() after mp_decode_extl()
+    ok 76 - decimal_unpack() after mp_decode_extl() value
+    ok 77 - decimal_unpack() after mp_decode_extl() len
+    ok 78 - mp_sizeof_decimal(-2.718281828459045)
+    ok 79 - mp_sizeof_decimal(-2.718281828459045) == len(mp_encode_decimal(-2.718281828459045))
+    ok 80 - mp_next(mp_encode(-2.718281828459045))
+    ok 81 - mp_decode(mp_encode(-2.718281828459045) len
+    ok 82 - mp_decode(mp_encode(-2.718281828459045)) value
+    ok 83 - mp_decode(mp_encode(-2.718281828459045)) scale
+    ok 84 - str(mp_decode(mp_encode(-2.718281828459045))) == -2.718281828459045
+    ok 85 - mp_ext_type is MP_DECIMAL
+    ok 86 - decimal_unpack() after mp_decode_extl()
+    ok 87 - decimal_unpack() after mp_decode_extl() value
+    ok 88 - decimal_unpack() after mp_decode_extl() len
+    ok 89 - mp_sizeof_decimal(3.141592653589793)
+    ok 90 - mp_sizeof_decimal(3.141592653589793) == len(mp_encode_decimal(3.141592653589793))
+    ok 91 - mp_next(mp_encode(3.141592653589793))
+    ok 92 - mp_decode(mp_encode(3.141592653589793) len
+    ok 93 - mp_decode(mp_encode(3.141592653589793)) value
+    ok 94 - mp_decode(mp_encode(3.141592653589793)) scale
+    ok 95 - str(mp_decode(mp_encode(3.141592653589793))) == 3.141592653589793
+    ok 96 - mp_ext_type is MP_DECIMAL
+    ok 97 - decimal_unpack() after mp_decode_extl()
+    ok 98 - decimal_unpack() after mp_decode_extl() value
+    ok 99 - decimal_unpack() after mp_decode_extl() len
+    ok 100 - mp_sizeof_decimal(-3.141592653589793)
+    ok 101 - mp_sizeof_decimal(-3.141592653589793) == len(mp_encode_decimal(-3.141592653589793))
+    ok 102 - mp_next(mp_encode(-3.141592653589793))
+    ok 103 - mp_decode(mp_encode(-3.141592653589793) len
+    ok 104 - mp_decode(mp_encode(-3.141592653589793)) value
+    ok 105 - mp_decode(mp_encode(-3.141592653589793)) scale
+    ok 106 - str(mp_decode(mp_encode(-3.141592653589793))) == -3.141592653589793
+    ok 107 - mp_ext_type is MP_DECIMAL
+    ok 108 - decimal_unpack() after mp_decode_extl()
+    ok 109 - decimal_unpack() after mp_decode_extl() value
+    ok 110 - decimal_unpack() after mp_decode_extl() len
+    ok 111 - mp_sizeof_decimal(1234567891234567890.0987654321987654321)
+    ok 112 - mp_sizeof_decimal(1234567891234567890.0987654321987654321) == len(mp_encode_decimal(1234567891234567890.0987654321987654321))
+    ok 113 - mp_next(mp_encode(1234567891234567890.0987654321987654321))
+    ok 114 - mp_decode(mp_encode(1234567891234567890.0987654321987654321) len
+    ok 115 - mp_decode(mp_encode(1234567891234567890.0987654321987654321)) value
+    ok 116 - mp_decode(mp_encode(1234567891234567890.0987654321987654321)) scale
+    ok 117 - str(mp_decode(mp_encode(1234567891234567890.0987654321987654321))) == 1234567891234567890.0987654321987654321
+    ok 118 - mp_ext_type is MP_DECIMAL
+    ok 119 - decimal_unpack() after mp_decode_extl()
+    ok 120 - decimal_unpack() after mp_decode_extl() value
+    ok 121 - decimal_unpack() after mp_decode_extl() len
+    ok 122 - mp_sizeof_decimal(-1234567891234567890.0987654321987654321)
+    ok 123 - mp_sizeof_decimal(-1234567891234567890.0987654321987654321) == len(mp_encode_decimal(-1234567891234567890.0987654321987654321))
+    ok 124 - mp_next(mp_encode(-1234567891234567890.0987654321987654321))
+    ok 125 - mp_decode(mp_encode(-1234567891234567890.0987654321987654321) len
+    ok 126 - mp_decode(mp_encode(-1234567891234567890.0987654321987654321)) value
+    ok 127 - mp_decode(mp_encode(-1234567891234567890.0987654321987654321)) scale
+    ok 128 - str(mp_decode(mp_encode(-1234567891234567890.0987654321987654321))) == -1234567891234567890.0987654321987654321
+    ok 129 - mp_ext_type is MP_DECIMAL
+    ok 130 - decimal_unpack() after mp_decode_extl()
+    ok 131 - decimal_unpack() after mp_decode_extl() value
+    ok 132 - decimal_unpack() after mp_decode_extl() len
+    ok 133 - mp_sizeof_decimal(0.0000000000000000000000000000000000001)
+    ok 134 - mp_sizeof_decimal(0.0000000000000000000000000000000000001) == len(mp_encode_decimal(0.0000000000000000000000000000000000001))
+    ok 135 - mp_next(mp_encode(0.0000000000000000000000000000000000001))
+    ok 136 - mp_decode(mp_encode(0.0000000000000000000000000000000000001) len
+    ok 137 - mp_decode(mp_encode(0.0000000000000000000000000000000000001)) value
+    ok 138 - mp_decode(mp_encode(0.0000000000000000000000000000000000001)) scale
+    ok 139 - str(mp_decode(mp_encode(0.0000000000000000000000000000000000001))) == 0.0000000000000000000000000000000000001
+    ok 140 - mp_ext_type is MP_DECIMAL
+    ok 141 - decimal_unpack() after mp_decode_extl()
+    ok 142 - decimal_unpack() after mp_decode_extl() value
+    ok 143 - decimal_unpack() after mp_decode_extl() len
+    ok 144 - mp_sizeof_decimal(-0.0000000000000000000000000000000000001)
+    ok 145 - mp_sizeof_decimal(-0.0000000000000000000000000000000000001) == len(mp_encode_decimal(-0.0000000000000000000000000000000000001))
+    ok 146 - mp_next(mp_encode(-0.0000000000000000000000000000000000001))
+    ok 147 - mp_decode(mp_encode(-0.0000000000000000000000000000000000001) len
+    ok 148 - mp_decode(mp_encode(-0.0000000000000000000000000000000000001)) value
+    ok 149 - mp_decode(mp_encode(-0.0000000000000000000000000000000000001)) scale
+    ok 150 - str(mp_decode(mp_encode(-0.0000000000000000000000000000000000001))) == -0.0000000000000000000000000000000000001
+    ok 151 - mp_ext_type is MP_DECIMAL
+    ok 152 - decimal_unpack() after mp_decode_extl()
+    ok 153 - decimal_unpack() after mp_decode_extl() value
+    ok 154 - decimal_unpack() after mp_decode_extl() len
+    ok 155 - mp_sizeof_decimal(0.00000000000000000000000000000000000001)
+    ok 156 - mp_sizeof_decimal(0.00000000000000000000000000000000000001) == len(mp_encode_decimal(0.00000000000000000000000000000000000001))
+    ok 157 - mp_next(mp_encode(0.00000000000000000000000000000000000001))
+    ok 158 - mp_decode(mp_encode(0.00000000000000000000000000000000000001) len
+    ok 159 - mp_decode(mp_encode(0.00000000000000000000000000000000000001)) value
+    ok 160 - mp_decode(mp_encode(0.00000000000000000000000000000000000001)) scale
+    ok 161 - str(mp_decode(mp_encode(0.00000000000000000000000000000000000001))) == 0.00000000000000000000000000000000000001
+    ok 162 - mp_ext_type is MP_DECIMAL
+    ok 163 - decimal_unpack() after mp_decode_extl()
+    ok 164 - decimal_unpack() after mp_decode_extl() value
+    ok 165 - decimal_unpack() after mp_decode_extl() len
+    ok 166 - mp_sizeof_decimal(-0.00000000000000000000000000000000000001)
+    ok 167 - mp_sizeof_decimal(-0.00000000000000000000000000000000000001) == len(mp_encode_decimal(-0.00000000000000000000000000000000000001))
+    ok 168 - mp_next(mp_encode(-0.00000000000000000000000000000000000001))
+    ok 169 - mp_decode(mp_encode(-0.00000000000000000000000000000000000001) len
+    ok 170 - mp_decode(mp_encode(-0.00000000000000000000000000000000000001)) value
+    ok 171 - mp_decode(mp_encode(-0.00000000000000000000000000000000000001)) scale
+    ok 172 - str(mp_decode(mp_encode(-0.00000000000000000000000000000000000001))) == -0.00000000000000000000000000000000000001
+    ok 173 - mp_ext_type is MP_DECIMAL
+    ok 174 - decimal_unpack() after mp_decode_extl()
+    ok 175 - decimal_unpack() after mp_decode_extl() value
+    ok 176 - decimal_unpack() after mp_decode_extl() len
+    ok 177 - mp_sizeof_decimal(99999999999999999999999999999999999999)
+    ok 178 - mp_sizeof_decimal(99999999999999999999999999999999999999) == len(mp_encode_decimal(99999999999999999999999999999999999999))
+    ok 179 - mp_next(mp_encode(99999999999999999999999999999999999999))
+    ok 180 - mp_decode(mp_encode(99999999999999999999999999999999999999) len
+    ok 181 - mp_decode(mp_encode(99999999999999999999999999999999999999)) value
+    ok 182 - mp_decode(mp_encode(99999999999999999999999999999999999999)) scale
+    ok 183 - str(mp_decode(mp_encode(99999999999999999999999999999999999999))) == 99999999999999999999999999999999999999
+    ok 184 - mp_ext_type is MP_DECIMAL
+    ok 185 - decimal_unpack() after mp_decode_extl()
+    ok 186 - decimal_unpack() after mp_decode_extl() value
+    ok 187 - decimal_unpack() after mp_decode_extl() len
+    ok 188 - mp_sizeof_decimal(-99999999999999999999999999999999999999)
+    ok 189 - mp_sizeof_decimal(-99999999999999999999999999999999999999) == len(mp_encode_decimal(-99999999999999999999999999999999999999))
+    ok 190 - mp_next(mp_encode(-99999999999999999999999999999999999999))
+    ok 191 - mp_decode(mp_encode(-99999999999999999999999999999999999999) len
+    ok 192 - mp_decode(mp_encode(-99999999999999999999999999999999999999)) value
+    ok 193 - mp_decode(mp_encode(-99999999999999999999999999999999999999)) scale
+    ok 194 - str(mp_decode(mp_encode(-99999999999999999999999999999999999999))) == -99999999999999999999999999999999999999
+    ok 195 - mp_ext_type is MP_DECIMAL
+    ok 196 - decimal_unpack() after mp_decode_extl()
+    ok 197 - decimal_unpack() after mp_decode_extl() value
+    ok 198 - decimal_unpack() after mp_decode_extl() len
+ok 280 - subtests
-- 
2.20.1 (Apple Git-117)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 3/5] lua: allow to encode and decode decimals as msgpack
  2019-07-17 15:33 [PATCH 0/5] Decimal indices Serge Petrenko
  2019-07-17 15:33 ` [PATCH 1/5] decimal: fix decimal.round() when scale == 0 Serge Petrenko
  2019-07-17 15:33 ` [PATCH 2/5] decimal: allow to encode/decode decimals as MsgPack Serge Petrenko
@ 2019-07-17 15:33 ` Serge Petrenko
  2019-07-24 14:11   ` Vladimir Davydov
  2019-07-24 23:10   ` [tarantool-patches] " Konstantin Osipov
  2019-07-17 15:33 ` [PATCH 4/5] decimal: add conversions to (u)int64_t Serge Petrenko
  2019-07-17 15:33 ` [PATCH 5/5] decimal: allow to index decimals Serge Petrenko
  4 siblings, 2 replies; 16+ messages in thread
From: Serge Petrenko @ 2019-07-17 15:33 UTC (permalink / raw)
  To: vdavydov.dev; +Cc: tarantool-patches, Serge Petrenko

It is now possible to insert decimals into spaces, but only into
unindexed fields.

Part-of #4333
---
 extra/exports                 |   4 ++
 src/lib/core/decimal.c        |  10 +++-
 src/lib/core/mp_user_types.h  |   3 +-
 src/lib/core/mpstream.c       |  11 ++++
 src/lib/core/mpstream.h       |   4 ++
 src/lua/decimal.c             |  10 ++--
 src/lua/decimal.h             |   5 ++
 src/lua/msgpack.c             |  52 +++++++++++++----
 src/lua/msgpackffi.lua        |  33 +++++++++++
 src/lua/utils.c               |  16 ++++-
 src/lua/utils.h               |   8 ++-
 test/app/decimal.result       | 106 +++++++++++++++++-----------------
 test/app/msgpack.result       |  41 +++++++++++++
 test/app/msgpack.test.lua     |  15 +++++
 third_party/lua-yaml/lyaml.cc |   6 ++
 15 files changed, 248 insertions(+), 76 deletions(-)

diff --git a/extra/exports b/extra/exports
index b8c42c0df..7b84a1452 100644
--- a/extra/exports
+++ b/extra/exports
@@ -62,8 +62,12 @@ PMurHash32_Result
 crc32_calc
 mp_encode_double
 mp_encode_float
+mp_encode_decimal
 mp_decode_double
 mp_decode_float
+mp_decode_extl
+mp_sizeof_decimal
+decimal_unpack
 
 log_type
 say_set_log_level
diff --git a/src/lib/core/decimal.c b/src/lib/core/decimal.c
index 1423ae418..2c69a773c 100644
--- a/src/lib/core/decimal.c
+++ b/src/lib/core/decimal.c
@@ -33,6 +33,7 @@
 #include "third_party/decNumber/decContext.h"
 #include "third_party/decNumber/decPacked.h"
 #include "lib/core/tt_static.h"
+#include "lib/msgpuck/msgpuck.h"
 #include <stddef.h>
 #include <stdlib.h>
 #include <float.h> /* DBL_DIG */
@@ -312,12 +313,15 @@ char *
 decimal_pack(char *data, const decimal_t *dec)
 {
 	uint32_t len = decimal_len(dec);
-	*data++ = decimal_scale(dec);
+	/* reserve space for resulting scale */
+	char *svp = data++;
 	len--;
 	int32_t scale;
 	char *tmp = (char *)decPackedFromNumber((uint8_t *)data, len, &scale, dec);
 	assert(tmp == data);
-	assert(scale == (int32_t)decimal_scale(dec));
+	/* scale may be negative, when exponent is > 0 */
+	assert(scale == (int32_t)decimal_scale(dec) || scale < 0);
+	mp_store_u8(svp, (int8_t)scale);
 	(void)tmp;
 	data += len;
 	return data;
@@ -326,7 +330,7 @@ decimal_pack(char *data, const decimal_t *dec)
 decimal_t *
 decimal_unpack(const char **data, uint32_t len, decimal_t *dec)
 {
-	int32_t scale = *((*data)++);
+	int32_t scale = (int8_t)mp_load_u8(data);
 	len--;
 	decimal_t *res = decPackedToNumber((uint8_t *)*data, len, &scale, dec);
 	if (res)
diff --git a/src/lib/core/mp_user_types.h b/src/lib/core/mp_user_types.h
index 9158b40d3..8211e3e79 100644
--- a/src/lib/core/mp_user_types.h
+++ b/src/lib/core/mp_user_types.h
@@ -32,7 +32,8 @@
  */
 
 enum mp_user_type {
-    MP_DECIMAL = 0
+    MP_UNKNOWN = 0,
+    MP_DECIMAL = 1
 };
 
 #endif
diff --git a/src/lib/core/mpstream.c b/src/lib/core/mpstream.c
index 8b7276ab1..a46b7962b 100644
--- a/src/lib/core/mpstream.c
+++ b/src/lib/core/mpstream.c
@@ -33,6 +33,7 @@
 #include <assert.h>
 #include <stdint.h>
 #include "msgpuck.h"
+#include "mp_decimal.h"
 
 void
 mpstream_reserve_slow(struct mpstream *stream, size_t size)
@@ -186,6 +187,16 @@ mpstream_encode_binl(struct mpstream *stream, uint32_t len)
 	mpstream_advance(stream, pos - data);
 }
 
+void
+mpstream_encode_decimal(struct mpstream *stream, decimal_t *val)
+{
+	char *data = mpstream_reserve(stream, mp_sizeof_decimal(val));
+	if (data == NULL)
+		return;
+	char *pos = mp_encode_decimal(data, val);
+	mpstream_advance(stream, pos - data);
+}
+
 void
 mpstream_memcpy(struct mpstream *stream, const void *src, uint32_t n)
 {
diff --git a/src/lib/core/mpstream.h b/src/lib/core/mpstream.h
index 69fa76f7f..44af28cb5 100644
--- a/src/lib/core/mpstream.h
+++ b/src/lib/core/mpstream.h
@@ -32,6 +32,7 @@
  */
 
 #include "diag.h"
+#include "decimal.h"
 
 #if defined(__cplusplus)
 extern "C" {
@@ -136,6 +137,9 @@ mpstream_encode_bool(struct mpstream *stream, bool val);
 void
 mpstream_encode_binl(struct mpstream *stream, uint32_t len);
 
+void
+mpstream_encode_decimal(struct mpstream *stream, decimal_t *val);
+
 /** Copies n bytes from memory area src to stream. */
 void
 mpstream_memcpy(struct mpstream *stream, const void *src, uint32_t n);
diff --git a/src/lua/decimal.c b/src/lua/decimal.c
index e548cdb9d..ab8d85f75 100644
--- a/src/lua/decimal.c
+++ b/src/lua/decimal.c
@@ -31,7 +31,7 @@
 
 #include "lua/decimal.h"
 #include "lib/core/decimal.h"
-#include "lua/utils.h"
+#include "lua/utils.h" /* CTID_DECIMAL, ... */
 
 #include <lua.h>
 #include <lauxlib.h>
@@ -69,16 +69,18 @@ ldecimal_##name(struct lua_State *L) {						\
 static int									\
 ldecimal_##name(struct lua_State *L) {						\
 	assert(lua_gettop(L) == 2);						\
+	if (lua_isnil(L, 1) || lua_isnil(L, 2)) {				\
+		lua_pushboolean(L, false);					\
+		return 1;							\
+	}									\
 	decimal_t *lhs = lua_todecimal(L, 1);					\
 	decimal_t *rhs = lua_todecimal(L, 2);					\
 	lua_pushboolean(L, decimal_compare(lhs, rhs) cmp 0);			\
 	return 1;								\
 }
 
-static uint32_t CTID_DECIMAL;
-
 /** Push a new decimal on the stack and return a pointer to it. */
-static decimal_t *
+decimal_t *
 lua_pushdecimal(struct lua_State *L)
 {
 	decimal_t *res = luaL_pushcdata(L, CTID_DECIMAL);
diff --git a/src/lua/decimal.h b/src/lua/decimal.h
index 0485d11ef..b5c0e54b4 100644
--- a/src/lua/decimal.h
+++ b/src/lua/decimal.h
@@ -31,12 +31,17 @@
 #ifndef TARANTOOL_LUA_DECIMAL_H_INCLUDED
 #define TARANTOOL_LUA_DECIMAL_H_INCLUDED
 
+#include "lib/core/decimal.h"
+
 #if defined(__cplusplus)
 extern "C" {
 #endif /* defined(__cplusplus) */
 
 struct lua_State;
 
+decimal_t *
+lua_pushdecimal(struct lua_State *L);
+
 void
 tarantool_lua_decimal_init(struct lua_State *L);
 
diff --git a/src/lua/msgpack.c b/src/lua/msgpack.c
index 2126988eb..3cb7d7dcd 100644
--- a/src/lua/msgpack.c
+++ b/src/lua/msgpack.c
@@ -41,6 +41,10 @@
 #include <small/region.h>
 #include <small/ibuf.h>
 
+#include "lua/decimal.h"
+#include "lib/core/decimal.h"
+#include "lib/core/mp_user_types.h"
+
 #include <fiber.h>
 
 void
@@ -175,16 +179,23 @@ restart: /* used by MP_EXT */
 		assert(lua_gettop(L) == top);
 		return MP_ARRAY;
 	case MP_EXT:
-		/* Run trigger if type can't be encoded */
-		type = luamp_encode_extension(L, top, stream);
-		if (type != MP_EXT)
-			return type; /* Value has been packed by the trigger */
-		/* Try to convert value to serializable type */
-		luaL_convertfield(L, cfg, top, field);
-		/* handled by luaL_convertfield */
-		assert(field->type != MP_EXT);
-		assert(lua_gettop(L) == top);
-		goto restart;
+		switch (field->ext_type)
+		{
+		case MP_DECIMAL:
+			mpstream_encode_decimal(stream, field->decval);
+			return MP_EXT;
+		case MP_UNKNOWN:
+			/* Run trigger if type can't be encoded */
+			type = luamp_encode_extension(L, top, stream);
+			if (type != MP_EXT)
+				return type; /* Value has been packed by the trigger */
+			/* Try to convert value to serializable type */
+			luaL_convertfield(L, cfg, top, field);
+			/* handled by luaL_convertfield */
+			assert(field->type != MP_EXT || field->ext_type != MP_UNKNOWN);
+			assert(lua_gettop(L) == top);
+			goto restart;
+		}
 	}
 	return MP_EXT;
 }
@@ -283,9 +294,28 @@ luamp_decode(struct lua_State *L, struct luaL_serializer *cfg,
 		return;
 	}
 	case MP_EXT:
-		luamp_decode_extension(L, data);
+	{
+		uint32_t len;
+		int8_t type;
+		len = mp_decode_extl(data, &type);
+		switch (type) {
+		case MP_DECIMAL:
+		{
+			decimal_t *dec = lua_pushdecimal(L);
+			dec = decimal_unpack(data, len, dec);
+			if (dec == NULL) {
+				lua_pop(L, -1);
+				luaL_error(L, "msgpack.decode: "
+					      "invalid MsgPack.");
+			}
+			return;
+		}
+		default:
+			luamp_decode_extension(L, data);
+		}
 		break;
 	}
+	}
 }
 
 
diff --git a/src/lua/msgpackffi.lua b/src/lua/msgpackffi.lua
index bfeedbc4b..4c799eed0 100644
--- a/src/lua/msgpackffi.lua
+++ b/src/lua/msgpackffi.lua
@@ -17,10 +17,18 @@ char *
 mp_encode_float(char *data, float num);
 char *
 mp_encode_double(char *data, double num);
+char *
+mp_encode_decimal(char *data, decimal_t *dec);
+uint32_t
+mp_sizeof_decimal(const decimal_t *dec);
 float
 mp_decode_float(const char **data);
 double
 mp_decode_double(const char **data);
+uint32_t
+mp_decode_extl(const char **data, int8_t *type);
+decimal_t *
+decimal_unpack(const char **data, uint32_t len, decimal_t *dec);
 ]])
 
 local strict_alignment = (jit.arch == 'arm')
@@ -117,6 +125,11 @@ local function encode_double(buf, num)
     builtin.mp_encode_double(p, num)
 end
 
+local function encode_decimal(buf, num)
+    local p = buf:alloc(builtin.mp_sizeof_decimal(num))
+    builtin.mp_encode_decimal(p, num)
+end
+
 local function encode_int(buf, num)
     if num >= 0 then
         if num <= 0x7f then
@@ -294,6 +307,7 @@ on_encode(ffi.typeof('const unsigned char'), encode_int)
 on_encode(ffi.typeof('bool'), encode_bool_cdata)
 on_encode(ffi.typeof('float'), encode_float)
 on_encode(ffi.typeof('double'), encode_double)
+on_encode(ffi.typeof('decimal_t'), encode_decimal)
 
 --------------------------------------------------------------------------------
 -- Decoder
@@ -473,6 +487,23 @@ local function decode_map(data, size)
     return setmetatable(map, msgpack.map_mt)
 end
 
+local function decode_ext(data)
+    local t = ffi.new("int8_t[1]")
+    -- mp_decode_extl and mp_decode_decimal
+    -- need type code
+    data[0] = data[0] - 1
+    local old_data = data[0]
+    local len = builtin.mp_decode_extl(data, t)
+    --MP_DECIMAL
+    if t[0] == 1 then
+        local num = ffi.new("decimal_t")
+        builtin.decimal_unpack(data, len, num)
+        return num
+    else
+        error("Unsupported extension type")
+    end
+end
+
 local decoder_hint = {
     --[[{{{ MP_BIN]]
     [0xc4] = function(data) return decode_str(data, decode_u8(data)) end;
@@ -528,6 +559,8 @@ decode_r = function(data)
         return false
     elseif c == 0xc3 then
         return true
+    elseif c >= 0xd4 and c <= 0xd8 or c >= 0xc7 and c <= 0xc9 then
+        return decode_ext(data)
     else
         local fun = decoder_hint[c];
         assert (type(fun) == "function")
diff --git a/src/lua/utils.c b/src/lua/utils.c
index 0a4bcf517..47cf030ab 100644
--- a/src/lua/utils.c
+++ b/src/lua/utils.c
@@ -45,6 +45,8 @@ static uint32_t CTID_STRUCT_IBUF;
 static uint32_t CTID_STRUCT_IBUF_PTR;
 uint32_t CTID_CHAR_PTR;
 uint32_t CTID_CONST_CHAR_PTR;
+uint32_t CTID_DECIMAL;
+
 
 void *
 luaL_pushcdata(struct lua_State *L, uint32_t ctypeid)
@@ -723,6 +725,12 @@ luaL_tofield(struct lua_State *L, struct luaL_serializer *cfg, int index,
 			/* Fall through */
 		default:
 			field->type = MP_EXT;
+			if (cd->ctypeid == CTID_DECIMAL) {
+				field->ext_type = MP_DECIMAL;
+				field->decval = (decimal_t *) cdata;
+			} else {
+				field->ext_type = MP_UNKNOWN;
+			}
 		}
 		return 0;
 	}
@@ -754,6 +762,7 @@ luaL_tofield(struct lua_State *L, struct luaL_serializer *cfg, int index,
 		/* Fall through */
 	default:
 		field->type = MP_EXT;
+		field->ext_type = MP_UNKNOWN;
 	}
 #undef CHECK_NUMBER
 	return 0;
@@ -765,7 +774,7 @@ luaL_convertfield(struct lua_State *L, struct luaL_serializer *cfg, int idx,
 {
 	if (idx < 0)
 		idx = lua_gettop(L) + idx + 1;
-	assert(field->type == MP_EXT); /* must be called after tofield() */
+	assert(field->type == MP_EXT && field->ext_type == MP_UNKNOWN); /* must be called after tofield() */
 
 	if (cfg->encode_load_metatables) {
 		int type = lua_type(L, idx);
@@ -782,10 +791,11 @@ luaL_convertfield(struct lua_State *L, struct luaL_serializer *cfg, int idx,
 		}
 	}
 
-	if (field->type == MP_EXT && cfg->encode_use_tostring)
+	if (field->type == MP_EXT && field->ext_type == MP_UNKNOWN &&
+	    cfg->encode_use_tostring)
 		lua_field_tostring(L, cfg, idx, field);
 
-	if (field->type != MP_EXT)
+	if (field->type != MP_EXT || field->ext_type != MP_UNKNOWN)
 		return;
 
 	if (cfg->encode_invalid_as_nil) {
diff --git a/src/lua/utils.h b/src/lua/utils.h
index 7e7cdc0c6..3ca43292b 100644
--- a/src/lua/utils.h
+++ b/src/lua/utils.h
@@ -54,6 +54,8 @@ extern "C" {
 #include <lj_meta.h>
 
 #include "lua/error.h"
+#include "lib/core/mp_user_types.h"
+#include "lib/core/decimal.h" /* decimal_t */
 
 struct lua_State;
 struct ibuf;
@@ -69,6 +71,8 @@ extern struct ibuf *tarantool_lua_ibuf;
 
 extern uint32_t CTID_CONST_CHAR_PTR;
 extern uint32_t CTID_CHAR_PTR;
+extern uint32_t CTID_DECIMAL;
+
 
 /** \cond public */
 
@@ -286,8 +290,10 @@ struct luaL_field {
 		bool bval;
 		/* Array or map. */
 		uint32_t size;
+		decimal_t *decval;
 	};
 	enum mp_type type;
+	enum mp_user_type ext_type;
 	bool compact;                /* a flag used by YAML serializer */
 };
 
@@ -373,7 +379,7 @@ luaL_checkfield(struct lua_State *L, struct luaL_serializer *cfg, int idx,
 {
 	if (luaL_tofield(L, cfg, idx, field) < 0)
 		luaT_error(L);
-	if (field->type != MP_EXT)
+	if (field->type != MP_EXT || field->ext_type != MP_UNKNOWN)
 		return;
 	luaL_convertfield(L, cfg, idx, field);
 }
diff --git a/test/app/decimal.result b/test/app/decimal.result
index 53ec73edc..334c9fbab 100644
--- a/test/app/decimal.result
+++ b/test/app/decimal.result
@@ -12,77 +12,77 @@ ffi = require('ffi')
 -- check various constructors
 decimal.new('1234.5678')
  | ---
- | - '1234.5678'
+ | - 1234.5678
  | ...
 decimal.new('1e6')
  | ---
- | - '1000000'
+ | - 1000000
  | ...
 decimal.new('-6.234612e2')
  | ---
- | - '-623.4612'
+ | - -623.4612
  | ...
 -- check (u)int16/32/64_t
 decimal.new(2ULL ^ 63)
  | ---
- | - '9223372036854775808'
+ | - 9223372036854775808
  | ...
 decimal.new(123456789123456789ULL)
  | ---
- | - '123456789123456789'
+ | - 123456789123456789
  | ...
 decimal.new(-123456789123456789LL)
  | ---
- | - '-123456789123456789'
+ | - -123456789123456789
  | ...
 decimal.new(ffi.new('uint8_t', 231))
  | ---
- | - '231'
+ | - 231
  | ...
 decimal.new(ffi.new('int8_t', -113))
  | ---
- | - '-113'
+ | - -113
  | ...
 decimal.new(ffi.new('uint16_t', 65535))
  | ---
- | - '65535'
+ | - 65535
  | ...
 decimal.new(ffi.new('int16_t', -31263))
  | ---
- | - '-31263'
+ | - -31263
  | ...
 decimal.new(ffi.new('uint32_t', 4123123123))
  | ---
- | - '4123123123'
+ | - 4123123123
  | ...
 decimal.new(ffi.new('int32_t', -2123123123))
  | ---
- | - '-2123123123'
+ | - -2123123123
  | ...
 decimal.new(ffi.new('float', 128.5))
  | ---
- | - '128.5'
+ | - 128.5
  | ...
 decimal.new(ffi.new('double', 128.5))
  | ---
- | - '128.5'
+ | - 128.5
  | ...
 
 decimal.new(1)
  | ---
- | - '1'
+ | - 1
  | ...
 decimal.new(-1)
  | ---
- | - '-1'
+ | - -1
  | ...
 decimal.new(2^64)
  | ---
- | - '18446744073709600000'
+ | - 18446744073709600000
  | ...
 decimal.new(2^(-20))
  | ---
- | - '0.00000095367431640625'
+ | - 0.00000095367431640625
  | ...
 
 -- incorrect constructions
@@ -128,38 +128,38 @@ a = decimal.new('10')
  | ...
 a
  | ---
- | - '10'
+ | - 10
  | ...
 b = decimal.new('0.1')
  | ---
  | ...
 b
  | ---
- | - '0.1'
+ | - 0.1
  | ...
 a + b
  | ---
- | - '10.1'
+ | - 10.1
  | ...
 a - b
  | ---
- | - '9.9'
+ | - 9.9
  | ...
 a * b
  | ---
- | - '1.0'
+ | - 1.0
  | ...
 a / b
  | ---
- | - '100'
+ | - 100
  | ...
 a ^ b
  | ---
- | - '1.2589254117941672104239541063958006061'
+ | - 1.2589254117941672104239541063958006061
  | ...
 b ^ a
  | ---
- | - '0.0000000001'
+ | - 0.0000000001
  | ...
 -a + -b == -(a + b)
  | ---
@@ -167,11 +167,11 @@ b ^ a
  | ...
 a
  | ---
- | - '10'
+ | - 10
  | ...
 b
  | ---
- | - '0.1'
+ | - 0.1
  | ...
 
 a < b
@@ -216,28 +216,28 @@ a ~= b
  | ...
 a
  | ---
- | - '10'
+ | - 10
  | ...
 b
  | ---
- | - '0.1'
+ | - 0.1
  | ...
 
 decimal.sqrt(a)
  | ---
- | - '3.1622776601683793319988935444327185337'
+ | - 3.1622776601683793319988935444327185337
  | ...
 decimal.ln(a)
  | ---
- | - '2.3025850929940456840179914546843642076'
+ | - 2.3025850929940456840179914546843642076
  | ...
 decimal.log10(a)
  | ---
- | - '1'
+ | - 1
  | ...
 decimal.exp(a)
  | ---
- | - '22026.465794806716516957900645284244366'
+ | - 22026.465794806716516957900645284244366
  | ...
 a == decimal.ln(decimal.exp(a))
  | ---
@@ -261,7 +261,7 @@ a + -a == 0
  | ...
 a
  | ---
- | - '10'
+ | - 10
  | ...
 
 a = decimal.new('1.1234567891234567891234567891234567891')
@@ -269,7 +269,7 @@ a = decimal.new('1.1234567891234567891234567891234567891')
  | ...
 a
  | ---
- | - '1.1234567891234567891234567891234567891'
+ | - 1.1234567891234567891234567891234567891
  | ...
 decimal.precision(a)
  | ---
@@ -285,7 +285,7 @@ decimal.round(a, 37) == a
  | ...
 a
  | ---
- | - '1.1234567891234567891234567891234567891'
+ | - 1.1234567891234567891234567891234567891
  | ...
 a = decimal.round(a, 36)
  | ---
@@ -309,19 +309,19 @@ decimal.round(a, -5) == a
  | ...
 decimal.round(a, 7)
  | ---
- | - '1.1234568'
+ | - 1.1234568
  | ...
 decimal.round(a, 3)
  | ---
- | - '1.123'
+ | - 1.123
  | ...
 decimal.round(a, 0)
  | ---
- | - '1'
+ | - 1
  | ...
 a
  | ---
- | - '1.123456789123456789123456789123456789'
+ | - 1.123456789123456789123456789123456789
  | ...
 
 decimal.ln(0)
@@ -334,7 +334,7 @@ decimal.ln(-1)
  | ...
 decimal.ln(1)
  | ---
- | - '0'
+ | - 0
  | ...
 decimal.log10(0)
  | ---
@@ -346,7 +346,7 @@ decimal.log10(-1)
  | ...
 decimal.log10(1)
  | ---
- | - '0'
+ | - 0
  | ...
 decimal.exp(88)
  | ---
@@ -354,7 +354,7 @@ decimal.exp(88)
  | ...
 decimal.exp(87)
  | ---
- | - '60760302250568721495223289381302760753'
+ | - 60760302250568721495223289381302760753
  | ...
 decimal.sqrt(-5)
  | ---
@@ -362,7 +362,7 @@ decimal.sqrt(-5)
  | ...
 decimal.sqrt(5)
  | ---
- | - '2.2360679774997896964091736687312762354'
+ | - 2.2360679774997896964091736687312762354
  | ...
 
 -- various incorrect operands
@@ -408,11 +408,11 @@ a ^ 2
  | ...
 a ^ 1.9
  | ---
- | - '1258925411794167210423954106395800606.1'
+ | - 1258925411794167210423954106395800606.1
  | ...
 a * '1e18'
  | ---
- | - '10000000000000000000000000000000000000'
+ | - 10000000000000000000000000000000000000
  | ...
 a = decimal.new(string.rep('9', 38))
  | ---
@@ -435,7 +435,7 @@ a + '0.5'
  | ...
 a + '0.4'
  | ---
- | - '99999999999999999999999999999999999999'
+ | - 99999999999999999999999999999999999999
  | ...
 a / 0.5
  | ---
@@ -451,7 +451,7 @@ a = decimal.new('-13')
  | ...
 a ^ 2
  | ---
- | - '169'
+ | - 169
  | ...
 -- fractional powers are allowed only for positive numbers
 a ^ 2.5
@@ -462,17 +462,17 @@ a ^ 2.5
 -- check correct rounding when scale = 0
 decimal.round(decimal.new(0.9), 0)
  | ---
- | - '1'
+ | - 1
  | ...
 decimal.round(decimal.new(9.9), 0)
  | ---
- | - '10'
+ | - 10
  | ...
 decimal.round(decimal.new(99.9), 0)
  | ---
- | - '100'
+ | - 100
  | ...
 decimal.round(decimal.new(99.4), 0)
  | ---
- | - '99'
+ | - 99
  | ...
diff --git a/test/app/msgpack.result b/test/app/msgpack.result
index a67c05d38..4b5aec784 100644
--- a/test/app/msgpack.result
+++ b/test/app/msgpack.result
@@ -252,3 +252,44 @@ msgpack.decode(ffi.cast('char *', '\x04\x05\x06'), -1)
 ---
 - error: 'msgpack.decode: size can''t be negative'
 ...
+--
+-- gh-4333: msgpack encode/decode decimals.
+--
+decimal = require('decimal')
+---
+...
+a = decimal.new('1e37')
+---
+...
+b = decimal.new('1e-38')
+---
+...
+c = decimal.new('1')
+---
+...
+d = decimal.new('0.1234567')
+---
+...
+e = decimal.new('123.4567')
+---
+...
+msgpack.decode(msgpack.encode(a)) == a
+---
+- true
+...
+msgpack.decode(msgpack.encode(b)) == b
+---
+- true
+...
+msgpack.decode(msgpack.encode(c)) == c
+---
+- true
+...
+msgpack.decode(msgpack.encode(d)) == d
+---
+- true
+...
+msgpack.decode(msgpack.encode(e)) == e
+---
+- true
+...
diff --git a/test/app/msgpack.test.lua b/test/app/msgpack.test.lua
index e0880ac22..9224d870a 100644
--- a/test/app/msgpack.test.lua
+++ b/test/app/msgpack.test.lua
@@ -84,3 +84,18 @@ size = msgpack.encode(100, buf)
 -- is not negative.
 --
 msgpack.decode(ffi.cast('char *', '\x04\x05\x06'), -1)
+
+--
+-- gh-4333: msgpack encode/decode decimals.
+--
+decimal = require('decimal')
+a = decimal.new('1e37')
+b = decimal.new('1e-38')
+c = decimal.new('1')
+d = decimal.new('0.1234567')
+e = decimal.new('123.4567')
+msgpack.decode(msgpack.encode(a)) == a
+msgpack.decode(msgpack.encode(b)) == b
+msgpack.decode(msgpack.encode(c)) == c
+msgpack.decode(msgpack.encode(d)) == d
+msgpack.decode(msgpack.encode(e)) == e
diff --git a/third_party/lua-yaml/lyaml.cc b/third_party/lua-yaml/lyaml.cc
index 46c98bde1..8ff2867e3 100644
--- a/third_party/lua-yaml/lyaml.cc
+++ b/third_party/lua-yaml/lyaml.cc
@@ -49,6 +49,7 @@ extern "C" {
 #include "b64.h"
 } /* extern "C" */
 #include "lua/utils.h"
+#include "lib/core/decimal.h"
 
 #define LUAYAML_TAG_PREFIX "tag:yaml.org,2002:"
 
@@ -693,6 +694,11 @@ static int dump_node(struct lua_yaml_dumper *dumper)
       len = 4;
       break;
    case MP_EXT:
+      if (field.ext_type == MP_DECIMAL) {
+         str = decimal_to_string(field.decval);
+	 len = strlen(str);
+         break;
+      }
       assert(0); /* checked by luaL_checkfield() */
       break;
     }
-- 
2.20.1 (Apple Git-117)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 4/5] decimal: add conversions to (u)int64_t
  2019-07-17 15:33 [PATCH 0/5] Decimal indices Serge Petrenko
                   ` (2 preceding siblings ...)
  2019-07-17 15:33 ` [PATCH 3/5] lua: allow to encode and decode decimals as msgpack Serge Petrenko
@ 2019-07-17 15:33 ` Serge Petrenko
  2019-07-24 16:17   ` Vladimir Davydov
  2019-07-17 15:33 ` [PATCH 5/5] decimal: allow to index decimals Serge Petrenko
  4 siblings, 1 reply; 16+ messages in thread
From: Serge Petrenko @ 2019-07-17 15:33 UTC (permalink / raw)
  To: vdavydov.dev; +Cc: tarantool-patches, Serge Petrenko

Update decNumber library, add methods to convert decimals to uint64_t
and int64_t, add unit tests.
Also replace decimal_round() function with decimal_round_with_mode() to
allow setting rounding mode.
We need to round with mode DEC_ROUND_DOWN in to_int64 conversions in
order to be consistent with double to int conversions.
It will be needed to compute hints for decimal fields.

Prerequisite #4333
---
 src/lib/core/decimal.c   | 63 ++++++++++++++++++++++++++++++++--
 src/lib/core/decimal.h   | 12 +++++++
 test/unit/decimal.c      | 66 ++++++++++++++++++++++++++++++++++-
 test/unit/decimal.result | 74 ++++++++++++++++++++++++++++++++++++++--
 third_party/decNumber    |  2 +-
 5 files changed, 209 insertions(+), 8 deletions(-)

diff --git a/src/lib/core/decimal.c b/src/lib/core/decimal.c
index 2c69a773c..5a576e6e5 100644
--- a/src/lib/core/decimal.c
+++ b/src/lib/core/decimal.c
@@ -157,6 +157,57 @@ decimal_to_string(const decimal_t *dec)
 	return buf;
 }
 
+static decimal_t *
+decimal_round_with_mode(decimal_t *dec, int scale, enum rounding mode);
+
+decimal_t *
+decimal_to_int64(decimal_t *dec, int64_t *num)
+{
+	decimal_t d, z;
+	decNumberZero(&z);
+	d = *dec;
+	dec = &d;
+
+	if (decimal_scale(dec) != 0) {
+		/*
+		 * Rounding mode is important here.
+		 * We want to be consistent with double
+		 * to int conversion so that comparison
+		 * hints work correctly.
+		 */
+		dec = decimal_round_with_mode(dec, 0, DEC_ROUND_DOWN);
+	}
+	/*
+	 * decNumberToInt64 works only with numbers with
+	 * zero exponent.
+	 */
+	decNumberRescale(dec, dec, &z, &decimal_context);
+	if (decimal_check_status(dec, &decimal_context) == NULL) {
+		return NULL;
+	}
+	*num = decNumberToInt64(dec, &decimal_context);
+	return decimal_check_status(dec, &decimal_context);
+}
+
+decimal_t *
+decimal_to_uint64(decimal_t *dec, uint64_t *num)
+{
+	decimal_t d, z;
+	decNumberZero(&z);
+	d = *dec;
+	dec = &d;
+
+	if (decimal_scale(dec) != 0) {
+		dec = decimal_round_with_mode(dec, 0, DEC_ROUND_DOWN);
+	}
+	decNumberRescale(dec, dec, &z, &decimal_context);
+	if (decimal_check_status(dec, &decimal_context) == NULL) {
+		return NULL;
+	}
+	*num = decNumberToUInt64(dec, &decimal_context);
+	return decimal_check_status(dec, &decimal_context);
+}
+
 int
 decimal_compare(const decimal_t *lhs, const decimal_t *rhs)
 {
@@ -167,8 +218,8 @@ decimal_compare(const decimal_t *lhs, const decimal_t *rhs)
 	return r;
 }
 
-decimal_t *
-decimal_round(decimal_t *dec, int scale)
+static decimal_t *
+decimal_round_with_mode(decimal_t *dec, int scale, enum rounding mode)
 {
 	if (scale < 0 || scale > DECIMAL_MAX_DIGITS)
 		return NULL;
@@ -181,7 +232,7 @@ decimal_round(decimal_t *dec, int scale)
 		ndig, /* Precision */
 		ndig, /* emax */
 		scale != 0 ? -1 : 0, /* emin */
-		DECIMAL_ROUNDING, /* rounding */
+		mode, /* rounding */
 		0, /* no traps */
 		0, /* zero status */
 		0 /* no clamping */
@@ -192,6 +243,12 @@ decimal_round(decimal_t *dec, int scale)
 	return dec;
 }
 
+inline decimal_t *
+decimal_round(decimal_t *dec, int scale)
+{
+	return decimal_round_with_mode(dec, scale, DECIMAL_ROUNDING);
+}
+
 decimal_t *
 decimal_abs(decimal_t *res, const decimal_t *dec)
 {
diff --git a/src/lib/core/decimal.h b/src/lib/core/decimal.h
index 1d0f2582e..a4e7683c7 100644
--- a/src/lib/core/decimal.h
+++ b/src/lib/core/decimal.h
@@ -101,6 +101,18 @@ decimal_from_uint64(decimal_t *dec, uint64_t num);
 const char *
 decimal_to_string(const decimal_t *dec);
 
+/**
+ * Convert a given decimal to int64_t
+ * \param[out] num - the result
+ * @return NULL if \a dec doesn't fit into int64_t
+ */
+decimal_t *
+decimal_to_int64(decimal_t *dec, int64_t *num);
+
+/** \sa decimal_to_int64 */
+decimal_t *
+decimal_to_uint64(decimal_t *dec, uint64_t *num);
+
 /**
  * Compare 2 decimal values.
  * @return -1, lhs < rhs,
diff --git a/test/unit/decimal.c b/test/unit/decimal.c
index dd64b95ed..6b609140b 100644
--- a/test/unit/decimal.c
+++ b/test/unit/decimal.c
@@ -5,6 +5,7 @@
 #include "msgpuck.h"
 #include <limits.h>
 #include <string.h>
+#include <inttypes.h>
 #include <float.h> /* DBL_DIG */
 
 #define success(x) x
@@ -117,6 +118,15 @@ char buf[32];
 	is(strcmp(decimal_to_string(&d2), str), 0, "str(decimal_unpack(decimal_pack("str")) == "str);\
 })
 
+#define test_toint(type, num, out_fmt) ({\
+	decimal_t dec;\
+	type##_t val;\
+	decimal_from_##type(&dec, num);\
+	isnt(decimal_to_##type(&dec, &val), NULL, "Conversion of %"out_fmt\
+						  " to decimal and back to "#type" successful", (type##_t) num);\
+	is(val, num, "Conversion back to "#type" correct");\
+})
+
 static int
 test_pack_unpack(void)
 {
@@ -181,10 +191,62 @@ test_mp_decimal(void)
 	return check_plan();
 }
 
+static int
+test_to_int(void)
+{
+	plan(66);
+
+	test_toint(uint64, ULLONG_MAX, PRIu64);
+	test_toint(int64, LLONG_MAX, PRId64);
+	test_toint(int64, LLONG_MIN, PRId64);
+	test_toint(uint64, 0, PRIu64);
+	test_toint(int64, 0, PRId64);
+	test_toint(int64, -1, PRId64);
+
+	/* test some arbitrary values. */
+	test_toint(uint64, ULLONG_MAX / 157, PRIu64);
+	test_toint(int64, LLONG_MAX / 157, PRId64);
+	test_toint(int64, LLONG_MIN / 157, PRId64);
+
+	test_toint(uint64, ULLONG_MAX / 157 / 151, PRIu64);
+	test_toint(int64, LLONG_MAX / 157 / 151, PRId64);
+	test_toint(int64, LLONG_MIN / 157 / 151, PRId64);
+
+	test_toint(uint64, ULLONG_MAX / 157 / 151 / 149, PRIu64);
+	test_toint(int64, LLONG_MAX / 157 / 151 / 149, PRId64);
+	test_toint(int64, LLONG_MIN / 157 / 151 / 149, PRId64);
+
+	test_toint(uint64, ULLONG_MAX / 157 / 151 / 149 / 139, PRIu64);
+	test_toint(int64, LLONG_MAX / 157 / 151 / 149 / 139, PRId64);
+	test_toint(int64, LLONG_MIN / 157 / 151 / 149 / 139, PRId64);
+
+	test_toint(uint64, ULLONG_MAX / 157 / 151 / 149 / 139 / 137, PRIu64);
+	test_toint(int64, LLONG_MAX / 156 / 151 / 149 / 139 / 137, PRId64);
+	test_toint(int64, LLONG_MIN / 156 / 151 / 149 / 139 / 137, PRId64);
+
+	test_toint(uint64, UINT_MAX, PRIu64);
+	test_toint(int64, INT_MAX, PRId64);
+	test_toint(int64, INT_MIN, PRId64);
+
+	test_toint(uint64, UINT_MAX / 157, PRIu64); /* ~ 27356479 */
+	test_toint(int64, INT_MAX / 157, PRId64);
+	test_toint(int64, INT_MIN / 157, PRId64);
+
+	test_toint(uint64, UINT_MAX / 157 / 151, PRIu64); /* ~ 181168 */
+	test_toint(int64, INT_MAX / 157 / 151, PRId64);
+	test_toint(int64, INT_MIN / 157 / 151, PRId64);
+
+	test_toint(uint64, UINT_MAX / 157 / 151 / 149, PRIu64); /* ~ 1215 */
+	test_toint(int64, INT_MAX / 157 / 151 / 149, PRId64);
+	test_toint(int64, INT_MIN / 157 / 151 / 149, PRId64);
+
+	return check_plan();
+}
+
 int
 main(void)
 {
-	plan(280);
+	plan(281);
 
 	dectest(314, 271, uint64, uint64_t);
 	dectest(65535, 23456, uint64, uint64_t);
@@ -244,6 +306,8 @@ main(void)
 	dectest_op1_fail(log10, -1);
 	dectest_op1_fail(sqrt, -10);
 
+	test_to_int();
+
 	test_pack_unpack();
 
 	test_mp_decimal();
diff --git a/test/unit/decimal.result b/test/unit/decimal.result
index 6d00996d3..5a9c804bc 100644
--- a/test/unit/decimal.result
+++ b/test/unit/decimal.result
@@ -1,4 +1,4 @@
-1..280
+1..281
 ok 1 - decimal(314)
 ok 2 - decimal(271)
 ok 3 - decimal(314) + decimal(271)
@@ -277,6 +277,74 @@ ok 275 - decimal_from_string(-1)
 ok 276 - decimal_log10(-1) - error on wrong operands.
 ok 277 - decimal_from_string(-10)
 ok 278 - decimal_sqrt(-10) - error on wrong operands.
+    1..66
+    ok 1 - Conversion of 18446744073709551615 to decimal and back to uint64 successful
+    ok 2 - Conversion back to uint64 correct
+    ok 3 - Conversion of 9223372036854775807 to decimal and back to int64 successful
+    ok 4 - Conversion back to int64 correct
+    ok 5 - Conversion of -9223372036854775808 to decimal and back to int64 successful
+    ok 6 - Conversion back to int64 correct
+    ok 7 - Conversion of 0 to decimal and back to uint64 successful
+    ok 8 - Conversion back to uint64 correct
+    ok 9 - Conversion of 0 to decimal and back to int64 successful
+    ok 10 - Conversion back to int64 correct
+    ok 11 - Conversion of -1 to decimal and back to int64 successful
+    ok 12 - Conversion back to int64 correct
+    ok 13 - Conversion of 117495185182863386 to decimal and back to uint64 successful
+    ok 14 - Conversion back to uint64 correct
+    ok 15 - Conversion of 58747592591431693 to decimal and back to int64 successful
+    ok 16 - Conversion back to int64 correct
+    ok 17 - Conversion of -58747592591431693 to decimal and back to int64 successful
+    ok 18 - Conversion back to int64 correct
+    ok 19 - Conversion of 778113809158035 to decimal and back to uint64 successful
+    ok 20 - Conversion back to uint64 correct
+    ok 21 - Conversion of 389056904579017 to decimal and back to int64 successful
+    ok 22 - Conversion back to int64 correct
+    ok 23 - Conversion of -389056904579017 to decimal and back to int64 successful
+    ok 24 - Conversion back to int64 correct
+    ok 25 - Conversion of 5222240329919 to decimal and back to uint64 successful
+    ok 26 - Conversion back to uint64 correct
+    ok 27 - Conversion of 2611120164959 to decimal and back to int64 successful
+    ok 28 - Conversion back to int64 correct
+    ok 29 - Conversion of -2611120164959 to decimal and back to int64 successful
+    ok 30 - Conversion back to int64 correct
+    ok 31 - Conversion of 37570074315 to decimal and back to uint64 successful
+    ok 32 - Conversion back to uint64 correct
+    ok 33 - Conversion of 18785037157 to decimal and back to int64 successful
+    ok 34 - Conversion back to int64 correct
+    ok 35 - Conversion of -18785037157 to decimal and back to int64 successful
+    ok 36 - Conversion back to int64 correct
+    ok 37 - Conversion of 274234119 to decimal and back to uint64 successful
+    ok 38 - Conversion back to uint64 correct
+    ok 39 - Conversion of 137996015 to decimal and back to int64 successful
+    ok 40 - Conversion back to int64 correct
+    ok 41 - Conversion of -137996015 to decimal and back to int64 successful
+    ok 42 - Conversion back to int64 correct
+    ok 43 - Conversion of 4294967295 to decimal and back to uint64 successful
+    ok 44 - Conversion back to uint64 correct
+    ok 45 - Conversion of 2147483647 to decimal and back to int64 successful
+    ok 46 - Conversion back to int64 correct
+    ok 47 - Conversion of -2147483648 to decimal and back to int64 successful
+    ok 48 - Conversion back to int64 correct
+    ok 49 - Conversion of 27356479 to decimal and back to uint64 successful
+    ok 50 - Conversion back to uint64 correct
+    ok 51 - Conversion of 13678239 to decimal and back to int64 successful
+    ok 52 - Conversion back to int64 correct
+    ok 53 - Conversion of -13678239 to decimal and back to int64 successful
+    ok 54 - Conversion back to int64 correct
+    ok 55 - Conversion of 181168 to decimal and back to uint64 successful
+    ok 56 - Conversion back to uint64 correct
+    ok 57 - Conversion of 90584 to decimal and back to int64 successful
+    ok 58 - Conversion back to int64 correct
+    ok 59 - Conversion of -90584 to decimal and back to int64 successful
+    ok 60 - Conversion back to int64 correct
+    ok 61 - Conversion of 1215 to decimal and back to uint64 successful
+    ok 62 - Conversion back to uint64 correct
+    ok 63 - Conversion of 607 to decimal and back to int64 successful
+    ok 64 - Conversion back to int64 correct
+    ok 65 - Conversion of -607 to decimal and back to int64 successful
+    ok 66 - Conversion back to int64 correct
+ok 279 - subtests
     1..146
     ok 1 - decimal_len(0)
     ok 2 - decimal_len(0) == len(decimal_pack(0)
@@ -424,7 +492,7 @@ ok 278 - decimal_sqrt(-10) - error on wrong operands.
     ok 144 - str(decimal_unpack(decimal_pack(-99999999999999999999999999999999999999)) == -99999999999999999999999999999999999999
     ok 145 - unpack malformed decimal fails
     ok 146 - decode malformed decimal preserves buffer position
-ok 279 - subtests
+ok 280 - subtests
     1..198
     ok 1 - mp_sizeof_decimal(0)
     ok 2 - mp_sizeof_decimal(0) == len(mp_encode_decimal(0))
@@ -624,4 +692,4 @@ ok 279 - subtests
     ok 196 - decimal_unpack() after mp_decode_extl()
     ok 197 - decimal_unpack() after mp_decode_extl() value
     ok 198 - decimal_unpack() after mp_decode_extl() len
-ok 280 - subtests
+ok 281 - subtests
diff --git a/third_party/decNumber b/third_party/decNumber
index ee540fca6..878ed752f 160000
--- a/third_party/decNumber
+++ b/third_party/decNumber
@@ -1 +1 @@
-Subproject commit ee540fca6a44b3a2df7258dc7a1ec612cbf84dce
+Subproject commit 878ed752f2453cd5e73587e67f7782aec9181a22
-- 
2.20.1 (Apple Git-117)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 5/5] decimal: allow to index decimals
  2019-07-17 15:33 [PATCH 0/5] Decimal indices Serge Petrenko
                   ` (3 preceding siblings ...)
  2019-07-17 15:33 ` [PATCH 4/5] decimal: add conversions to (u)int64_t Serge Petrenko
@ 2019-07-17 15:33 ` Serge Petrenko
  2019-07-24 16:56   ` Vladimir Davydov
                     ` (2 more replies)
  4 siblings, 3 replies; 16+ messages in thread
From: Serge Petrenko @ 2019-07-17 15:33 UTC (permalink / raw)
  To: vdavydov.dev; +Cc: tarantool-patches, Serge Petrenko

Indices can now be built over decimal fields.
A new field type - 'decimal' is introduced.
Decimal values may be stored in 'decimal' columns, as well as in
'scalar' and 'any' columns.

Closes #4333

@TarantoolBot document
Title: Document decimal field type.

Decimals may now be stored in spaces. A corresponding field type is
introduced: 'decimal'. Decimal values are also allowed in 'scalar' and
'number' fields.

'decimal' field type is appropriate for both memtx HASH and TREE
indices, as well as for vinyl TREE index.
---
 src/box/field_def.c          |  43 +++++--
 src/box/field_def.h          |  16 ++-
 src/box/key_def.h            |   2 +-
 src/box/tuple_compare.cc     | 160 ++++++++++++++++++++++++-
 src/box/tuple_format.c       |   2 +-
 src/lib/core/decimal.h       |   8 ++
 src/lib/core/mp_decimal.h    |   8 ++
 test/engine/decimal.result   | 226 +++++++++++++++++++++++++++++++++++
 test/engine/decimal.test.lua |  65 ++++++++++
 9 files changed, 510 insertions(+), 20 deletions(-)
 create mode 100644 test/engine/decimal.result
 create mode 100644 test/engine/decimal.test.lua

diff --git a/src/box/field_def.c b/src/box/field_def.c
index 346042b98..da06e6bde 100644
--- a/src/box/field_def.c
+++ b/src/box/field_def.c
@@ -52,17 +52,32 @@ const uint32_t field_mp_type[] = {
 	/* [FIELD_TYPE_UNSIGNED] =  */ 1U << MP_UINT,
 	/* [FIELD_TYPE_STRING]   =  */ 1U << MP_STR,
 	/* [FIELD_TYPE_NUMBER]   =  */ (1U << MP_UINT) | (1U << MP_INT) |
-		(1U << MP_FLOAT) | (1U << MP_DOUBLE),
+		(1U << MP_FLOAT) | (1U << MP_DOUBLE) | (1U << MP_EXT),
 	/* [FIELD_TYPE_INTEGER]  =  */ (1U << MP_UINT) | (1U << MP_INT),
 	/* [FIELD_TYPE_BOOLEAN]  =  */ 1U << MP_BOOL,
 	/* [FIELD_TYPE_VARBINARY] =  */ 1U << MP_BIN,
 	/* [FIELD_TYPE_SCALAR]   =  */ (1U << MP_UINT) | (1U << MP_INT) |
 		(1U << MP_FLOAT) | (1U << MP_DOUBLE) | (1U << MP_STR) |
-		(1U << MP_BIN) | (1U << MP_BOOL),
+		(1U << MP_BIN) | (1U << MP_BOOL) | (1U << MP_EXT),
+	/* [FIELD_TYPE_DECIMAL]  =  */ 1U << MP_EXT,
 	/* [FIELD_TYPE_ARRAY]    =  */ 1U << MP_ARRAY,
 	/* [FIELD_TYPE_MAP]      =  */ (1U << MP_MAP),
 };
 
+const uint32_t field_ext_type[] = {
+	/* [FIELD_TYPE_ANY]      =  */ UINT32_MAX & ~(1U << MP_UNKNOWN),
+	/* [FIELD_TYPE_UNSIGNED] =  */ 0,
+	/* [FIELD_TYPE_STRING]   =  */ 0,
+	/* [FIELD_TYPE_NUMBER]   =  */ 1U << MP_DECIMAL,
+	/* [FIELD_TYPE_INTEGER]  =  */ 0,
+	/* [FIELD_TYPE_BOOLEAN]  =  */ 0,
+	/* [FIELD_TYPE_VARBINARY] = */ 0,
+	/* [FIELD_TYPE_SCALAR]   =  */ 1U << MP_DECIMAL,
+	/* [FIELD_TYPE_DECIMAL]  =  */ 1U << MP_DECIMAL,
+	/* [FIELD_TYPE_ARRAY]    =  */ 0,
+	/* [FIELD_TYPE_MAP]      =  */ 0,
+};
+
 const char *field_type_strs[] = {
 	/* [FIELD_TYPE_ANY]      = */ "any",
 	/* [FIELD_TYPE_UNSIGNED] = */ "unsigned",
@@ -72,6 +87,7 @@ const char *field_type_strs[] = {
 	/* [FIELD_TYPE_BOOLEAN]  = */ "boolean",
 	/* [FIELD_TYPE_VARBINARY] = */"varbinary",
 	/* [FIELD_TYPE_SCALAR]   = */ "scalar",
+	/* [FIELD_TYPE_DECIMAL]  = */ "decimal",
 	/* [FIELD_TYPE_ARRAY]    = */ "array",
 	/* [FIELD_TYPE_MAP]      = */ "map",
 };
@@ -98,17 +114,18 @@ field_type_by_name_wrapper(const char *str, uint32_t len)
  * values can be stored in the j type.
  */
 static const bool field_type_compatibility[] = {
-	   /*   ANY   UNSIGNED  STRING   NUMBER  INTEGER  BOOLEAN VARBINARY SCALAR   ARRAY     MAP */
-/*   ANY    */ true,   false,   false,   false,   false,   false,   false,  false,   false,   false,
-/* UNSIGNED */ true,   true,    false,   true,    true,    false,   false,  true,    false,   false,
-/*  STRING  */ true,   false,   true,    false,   false,   false,   false,  true,    false,   false,
-/*  NUMBER  */ true,   false,   false,   true,    false,   false,   false,  true,    false,   false,
-/*  INTEGER */ true,   false,   false,   true,    true,    false,   false,  true,    false,   false,
-/*  BOOLEAN */ true,   false,   false,   false,   false,   true,    false,  true,    false,   false,
-/* VARBINARY*/ true,   false,   false,   false,   false,   false,   true,   true,    false,   false,
-/*  SCALAR  */ true,   false,   false,   false,   false,   false,   false,  true,    false,   false,
-/*   ARRAY  */ true,   false,   false,   false,   false,   false,   false,  false,   true,    false,
-/*    MAP   */ true,   false,   false,   false,   false,   false,   false,  false,   false,   true,
+	   /*   ANY   UNSIGNED  STRING   NUMBER  INTEGER  BOOLEAN VARBINARY SCALAR  DECIMAL  ARRAY    MAP  */
+/*   ANY    */ true,   false,   false,   false,   false,   false,   false,  false,  false,  false,   false,
+/* UNSIGNED */ true,   true,    false,   true,    true,    false,   false,  true,   false,  false,   false,
+/*  STRING  */ true,   false,   true,    false,   false,   false,   false,  true,   false,  false,   false,
+/*  NUMBER  */ true,   false,   false,   true,    false,   false,   false,  true,   false,  false,   false,
+/*  INTEGER */ true,   false,   false,   true,    true,    false,   false,  true,   false,  false,   false,
+/*  BOOLEAN */ true,   false,   false,   false,   false,   true,    false,  true,   false,  false,   false,
+/* VARBINARY*/ true,   false,   false,   false,   false,   false,   true,   true,   false,  false,   false,
+/*  SCALAR  */ true,   false,   false,   false,   false,   false,   false,  true,   false,  false,   false,
+/*  DECIMAL */ true,   false,   false,   true,    false,   false,   false,  true,   true,   false,   false,
+/*   ARRAY  */ true,   false,   false,   false,   false,   false,   false,  false,  false,  true,    false,
+/*    MAP   */ true,   false,   false,   false,   false,   false,   false,  false,  false,  false,   true,
 };
 
 bool
diff --git a/src/box/field_def.h b/src/box/field_def.h
index c1a7ec0a9..10dc9fc13 100644
--- a/src/box/field_def.h
+++ b/src/box/field_def.h
@@ -37,6 +37,7 @@
 #include <limits.h>
 #include <msgpuck.h>
 #include "opt_def.h"
+#include "lib/core/mp_user_types.h"
 
 #if defined(__cplusplus)
 extern "C" {
@@ -58,6 +59,7 @@ enum field_type {
 	FIELD_TYPE_BOOLEAN,
 	FIELD_TYPE_VARBINARY,
 	FIELD_TYPE_SCALAR,
+	FIELD_TYPE_DECIMAL,
 	FIELD_TYPE_ARRAY,
 	FIELD_TYPE_MAP,
 	field_type_MAX
@@ -109,8 +111,9 @@ field_type_by_name(const char *name, size_t len);
 /* MsgPack type names */
 extern const char *mp_type_strs[];
 
-/** A helper table for field_mp_type_is_compatible */
+/** Two helper tables for field_mp_type_is_compatible */
 extern const uint32_t field_mp_type[];
+extern const uint32_t field_ext_type[];
 
 extern const struct opt_def field_def_reg[];
 extern const struct field_def field_def_default;
@@ -142,15 +145,20 @@ struct field_def {
 	struct Expr *default_value_expr;
 };
 
-/** Checks if mp_type (MsgPack) is compatible with field type. */
+/** Checks if mp_typeof(data) is compatible with field type. */
 static inline bool
-field_mp_type_is_compatible(enum field_type type, enum mp_type mp_type,
+field_mp_type_is_compatible(enum field_type type, const char *data,
 			    bool is_nullable)
 {
+	enum mp_type mp_type = mp_typeof(*data);
+	int8_t ext_type = MP_UNKNOWN;
 	assert(type < field_type_MAX);
 	assert((size_t)mp_type < CHAR_BIT * sizeof(*field_mp_type));
 	uint32_t mask = field_mp_type[type] | (is_nullable * (1U << MP_NIL));
-	return (mask & (1U << mp_type)) != 0;
+	if (mp_type != MP_EXT)
+		return (mask & (1U << mp_type)) != 0;
+	mp_decode_extl(&data, &ext_type);
+	return (field_ext_type[type] & (1U << ext_type)) != 0;
 }
 
 static inline bool
diff --git a/src/box/key_def.h b/src/box/key_def.h
index f4a1a8fd1..ed3354ade 100644
--- a/src/box/key_def.h
+++ b/src/box/key_def.h
@@ -505,7 +505,7 @@ static inline int
 key_part_validate(enum field_type key_type, const char *key,
 		  uint32_t field_no, bool is_nullable)
 {
-	if (unlikely(!field_mp_type_is_compatible(key_type, mp_typeof(*key),
+	if (unlikely(!field_mp_type_is_compatible(key_type, key,
 						  is_nullable))) {
 		diag_set(ClientError, ER_KEY_PART_TYPE, field_no,
 			 field_type_strs[key_type]);
diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc
index 95a0f58c9..6ab28f662 100644
--- a/src/box/tuple_compare.cc
+++ b/src/box/tuple_compare.cc
@@ -33,6 +33,9 @@
 #include "coll/coll.h"
 #include "trivia/util.h" /* NOINLINE */
 #include <math.h>
+#include "lib/core/decimal.h"
+#include "lib/core/mp_decimal.h"
+#include "lib/core/mp_user_types.h"
 
 /* {{{ tuple_compare */
 
@@ -87,7 +90,7 @@ static enum mp_class mp_classes[] = {
 	/* .MP_BOOL    = */ MP_CLASS_BOOL,
 	/* .MP_FLOAT   = */ MP_CLASS_NUMBER,
 	/* .MP_DOUBLE  = */ MP_CLASS_NUMBER,
-	/* .MP_BIN     = */ MP_CLASS_BIN
+	/* .MP_EXT     = */ MP_CLASS_NUMBER /* requires additional parsing */
 };
 
 #define COMPARE_RESULT(a, b) (a < b ? -1 : a > b)
@@ -264,6 +267,50 @@ mp_compare_double_any_number(double lhs, const char *rhs,
 	return k * COMPARE_RESULT(lqbit, rqbit);
 }
 
+static int
+mp_compare_decimal_any_number(decimal_t *lhs, const char *rhs,
+			      enum mp_type rhs_type, int k)
+{
+	decimal_t rhs_dec;
+	switch (rhs_type) {
+	case MP_FLOAT:
+	{
+		double d = mp_decode_float(&rhs);
+		decimal_from_double(&rhs_dec, d);
+		break;
+	}
+	case MP_DOUBLE:
+	{
+		double d = mp_decode_double(&rhs);
+		decimal_from_double(&rhs_dec, d);
+		break;
+	}
+	case MP_INT:
+	{
+		int64_t num = mp_decode_int(&rhs);
+		decimal_from_int64(&rhs_dec, num);
+		break;
+	}
+	case MP_UINT:
+	{
+		uint64_t num = mp_decode_uint(&rhs);
+		decimal_from_uint64(&rhs_dec, num);
+		break;
+	}
+	case MP_EXT:
+	{
+		int8_t ext_type;
+		uint32_t len = mp_decode_extl(&rhs, &ext_type);
+		assert(ext_type == MP_DECIMAL);
+		decimal_unpack(&rhs, len, &rhs_dec);
+		break;
+	}
+	default:
+		unreachable();
+	}
+	return k * decimal_compare(lhs, &rhs_dec);
+}
+
 static int
 mp_compare_number_with_type(const char *lhs, enum mp_type lhs_type,
 			    const char *rhs, enum mp_type rhs_type)
@@ -271,6 +318,21 @@ mp_compare_number_with_type(const char *lhs, enum mp_type lhs_type,
 	assert(mp_classof(lhs_type) == MP_CLASS_NUMBER);
 	assert(mp_classof(rhs_type) == MP_CLASS_NUMBER);
 
+	/*
+	 * test decimals first, so that we don't have to
+	 * account for them in other comarators.
+	 */
+	decimal_t dec;
+	if (rhs_type == MP_EXT) {
+		return mp_compare_decimal_any_number(
+			mp_decode_decimal(&rhs, &dec), lhs, lhs_type, -1
+		);
+	}
+	if (lhs_type == MP_EXT) {
+		return mp_compare_decimal_any_number(
+			mp_decode_decimal(&lhs, &dec), rhs, rhs_type, 1
+		);
+	}
 	if (rhs_type == MP_FLOAT) {
 		return mp_compare_double_any_number(
 			mp_decode_float(&rhs), lhs, lhs_type, -1
@@ -410,6 +472,8 @@ tuple_compare_field(const char *field_a, const char *field_b,
 		return coll != NULL ?
 		       mp_compare_scalar_coll(field_a, field_b, coll) :
 		       mp_compare_scalar(field_a, field_b);
+	case FIELD_TYPE_DECIMAL:
+		return mp_compare_number(field_a, field_b);
 	default:
 		unreachable();
 		return 0;
@@ -443,6 +507,8 @@ tuple_compare_field_with_type(const char *field_a, enum mp_type a_type,
 		       mp_compare_scalar_coll(field_a, field_b, coll) :
 		       mp_compare_scalar_with_type(field_a, a_type,
 						   field_b, b_type);
+	case FIELD_TYPE_DECIMAL:
+		return mp_compare_number(field_a, field_b);
 	default:
 		unreachable();
 		return 0;
@@ -1356,6 +1422,25 @@ static const comparator_with_key_signature cmp_wk_arr[] = {
 #define HINT_VALUE_DOUBLE_MAX	(exp2(HINT_VALUE_BITS - 1) - 1)
 #define HINT_VALUE_DOUBLE_MIN	(-exp2(HINT_VALUE_BITS - 1))
 
+/**
+ * Max and min decimal numbers whose integral parts fit
+ * in a hint value.
+ */
+static const decimal_t HINT_VALUE_DECIMAL_MAX = {
+	18,				/* decimal digits */
+	0,				/* exponent */
+	0,				/* no special bits. */
+	{487, 423, 303, 752, 460, 576}	/* 576,460,752,303,423,488 = 2^59 - 1 */
+					/* 59 == HINT_VALUE_BITS - 1 */
+};
+
+static const decimal_t HINT_VALUE_DECIMAL_MIN = {
+	18,				/* decimal digits */
+	0,				/* exponent */
+	0x80,				/* negative bit */
+	{488, 423, 303, 752, 460, 576}	/* 576,460,752,303,423,488 = 2^59 */
+};
+
 /*
  * HINT_CLASS_BITS should be big enough to store any mp_class value.
  * Note, ((1 << HINT_CLASS_BITS) - 1) is reserved for HINT_NONE.
@@ -1415,6 +1500,25 @@ hint_double(double d)
 	return hint_create(MP_CLASS_NUMBER, val);
 }
 
+static inline hint_t
+hint_decimal(decimal_t *dec)
+{
+	uint64_t val = 0;
+	int64_t num;
+	if (decimal_compare(dec, &HINT_VALUE_DECIMAL_MAX) >= 0)
+		val = HINT_VALUE_MAX;
+	else if (decimal_compare(dec, &HINT_VALUE_DECIMAL_MIN) <= 0)
+		val = 0;
+	else {
+		dec = decimal_to_int64(dec, &num);
+		/* We've checked boundaries above. */
+		assert(dec != NULL);
+		val = num - HINT_VALUE_INT_MIN;
+	}
+
+	return hint_create(MP_CLASS_NUMBER, val);
+}
+
 static inline uint64_t
 hint_str_raw(const char *s, uint32_t len)
 {
@@ -1491,12 +1595,42 @@ field_hint_number(const char *field)
 		return hint_double(mp_decode_float(&field));
 	case MP_DOUBLE:
 		return hint_double(mp_decode_double(&field));
+	case MP_EXT:
+	{
+		int8_t ext_type;
+		uint32_t len = mp_decode_extl(&field, &ext_type);
+		switch(ext_type) {
+		case MP_DECIMAL:
+		{
+			decimal_t dec;
+			decimal_t *res;
+			/*
+			 * The effect of mp_decode_extl() +
+			 * decimal_unpack() is the same that
+			 * the one of mp_decode_decimal().
+			 */
+			res = decimal_unpack(&field, len, &dec);
+			assert(res != NULL);
+			return hint_decimal(res);
+		}
+		default:
+			unreachable();
+		}
+	}
 	default:
 		unreachable();
 	}
 	return HINT_NONE;
 }
 
+static inline hint_t
+field_hint_decimal(const char *field)
+{
+	assert(mp_typeof(*field) == MP_EXT);
+	decimal_t dec;
+	return hint_decimal(mp_decode_decimal(&field, &dec));
+}
+
 static inline hint_t
 field_hint_string(const char *field, struct coll *coll)
 {
@@ -1536,6 +1670,25 @@ field_hint_scalar(const char *field, struct coll *coll)
 	case MP_BIN:
 		len = mp_decode_binl(&field);
 		return hint_bin(field, len);
+	case MP_EXT:
+	{
+		int8_t ext_type;
+		uint32_t len = mp_decode_extl(&field, &ext_type);
+		switch(ext_type) {
+		case MP_DECIMAL:
+		{
+			decimal_t dec;
+			/*
+			 * The effect of mp_decode_extl() +
+			 * decimal_unpack() is the same that
+			 * the one of mp_decode_decimal().
+			 */
+			return hint_decimal(decimal_unpack(&field, len, &dec));
+		}
+		default:
+			unreachable();
+		}
+	}
 	default:
 		unreachable();
 	}
@@ -1563,6 +1716,8 @@ field_hint(const char *field, struct coll *coll)
 		return field_hint_varbinary(field);
 	case FIELD_TYPE_SCALAR:
 		return field_hint_scalar(field, coll);
+	case FIELD_TYPE_DECIMAL:
+		return field_hint_decimal(field);
 	default:
 		unreachable();
 	}
@@ -1668,6 +1823,9 @@ key_def_set_hint_func(struct key_def *def)
 	case FIELD_TYPE_SCALAR:
 		key_def_set_hint_func<FIELD_TYPE_SCALAR>(def);
 		break;
+	case FIELD_TYPE_DECIMAL:
+		key_def_set_hint_func<FIELD_TYPE_DECIMAL>(def);
+		break;
 	default:
 		/* Invalid key definition. */
 		def->key_hint = NULL;
diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
index 02fadf1cf..5bb659b87 100644
--- a/src/box/tuple_format.c
+++ b/src/box/tuple_format.c
@@ -1165,7 +1165,7 @@ tuple_format_iterator_next(struct tuple_format_iterator *it,
 	 * defined in format.
 	 */
 	bool is_nullable = tuple_field_is_nullable(field);
-	if (!field_mp_type_is_compatible(field->type, mp_typeof(*entry->data),
+	if (!field_mp_type_is_compatible(field->type, entry->data,
 					 is_nullable) != 0) {
 		diag_set(ClientError, ER_FIELD_TYPE,
 			 tuple_field_path(field),
diff --git a/src/lib/core/decimal.h b/src/lib/core/decimal.h
index a4e7683c7..6a152b08c 100644
--- a/src/lib/core/decimal.h
+++ b/src/lib/core/decimal.h
@@ -37,6 +37,10 @@
 #include "third_party/decNumber/decNumber.h"
 #include <stdint.h>
 
+#if defined(__cplusplus)
+extern "C" {
+#endif /* defined(__cplusplus) */
+
 typedef decNumber decimal_t;
 
 /**
@@ -207,4 +211,8 @@ decimal_pack(char *data, const decimal_t *dec);
 decimal_t *
 decimal_unpack(const char **data, uint32_t len, decimal_t *dec);
 
+#if defined(__cplusplus)
+} /* extern "C" */
+#endif /* defined(__cplusplus) */
+
 #endif /* TARANTOOL_LIB_CORE_DECIMAL_H_INCLUDED */
diff --git a/src/lib/core/mp_decimal.h b/src/lib/core/mp_decimal.h
index a991a5f16..778529068 100644
--- a/src/lib/core/mp_decimal.h
+++ b/src/lib/core/mp_decimal.h
@@ -34,6 +34,10 @@
 #include "decimal.h"
 #include <stdint.h>
 
+#if defined(__cplusplus)
+extern "C" {
+#endif /* defined(__cplusplus) */
+
 /**
  * \brief Calculate exact buffer size needed to store a decimal
  * pointed to by \a dec.
@@ -59,4 +63,8 @@ mp_decode_decimal(const char **data, decimal_t *dec);
 char *
 mp_encode_decimal(char *data, const decimal_t *dec);
 
+#if defined(__cplusplus)
+} /* extern "C" */
+#endif /* defined(__cplusplus) */
+
 #endif
diff --git a/test/engine/decimal.result b/test/engine/decimal.result
new file mode 100644
index 000000000..16cd335e0
--- /dev/null
+++ b/test/engine/decimal.result
@@ -0,0 +1,226 @@
+-- test-run result file version 2
+env = require('test_run')
+ | ---
+ | ...
+test_run = env.new()
+ | ---
+ | ...
+engine = test_run:get_cfg('engine')
+ | ---
+ | ...
+
+decimal = require('decimal')
+ | ---
+ | ...
+ffi = require('ffi')
+ | ---
+ | ...
+
+_ = box.schema.space.create('test', {engine=engine})
+ | ---
+ | ...
+_ = box.space.test:create_index('pk', {parts={1,'decimal'}})
+ | ---
+ | ...
+
+test_run:cmd('setopt delimiter ";"')
+ | ---
+ | - true
+ | ...
+for i = 0,16 do
+    box.space.test:insert{decimal.new((i-8)/4)}
+end;
+ | ---
+ | ...
+test_run:cmd('setopt delimiter ""');
+ | ---
+ | - true
+ | ...
+
+box.space.test:select{}
+ | ---
+ | - - [-2]
+ |   - [-1.75]
+ |   - [-1.5]
+ |   - [-1.25]
+ |   - [-1]
+ |   - [-0.75]
+ |   - [-0.5]
+ |   - [-0.25]
+ |   - [0]
+ |   - [0.25]
+ |   - [0.5]
+ |   - [0.75]
+ |   - [1]
+ |   - [1.25]
+ |   - [1.5]
+ |   - [1.75]
+ |   - [2]
+ | ...
+
+-- check invalid values
+box.space.test:insert{1.23}
+ | ---
+ | - error: 'Tuple field 1 type does not match one required by operation: expected decimal'
+ | ...
+box.space.test:insert{'str'}
+ | ---
+ | - error: 'Tuple field 1 type does not match one required by operation: expected decimal'
+ | ...
+box.space.test:insert{ffi.new('uint64_t', 0)}
+ | ---
+ | - error: 'Tuple field 1 type does not match one required by operation: expected decimal'
+ | ...
+-- check duplicates
+box.space.test:insert{decimal.new(0)}
+ | ---
+ | - error: Duplicate key exists in unique index 'pk' in space 'test'
+ | ...
+
+box.space.test.index.pk:drop()
+ | ---
+ | ...
+
+_ = box.space.test:create_index('pk', {parts={1, 'number'}})
+ | ---
+ | ...
+
+test_run:cmd('setopt delimiter ";"')
+ | ---
+ | - true
+ | ...
+for i = 0, 32 do
+    local val = (i - 16) / 8
+    if i % 2 == 1 then val = decimal.new(val) end
+    box.space.test:insert{val}
+end;
+ | ---
+ | ...
+test_run:cmd('setopt delimiter ""');
+ | ---
+ | - true
+ | ...
+
+box.space.test:select{}
+ | ---
+ | - - [-2]
+ |   - [-1.875]
+ |   - [-1.75]
+ |   - [-1.625]
+ |   - [-1.5]
+ |   - [-1.375]
+ |   - [-1.25]
+ |   - [-1.125]
+ |   - [-1]
+ |   - [-0.875]
+ |   - [-0.75]
+ |   - [-0.625]
+ |   - [-0.5]
+ |   - [-0.375]
+ |   - [-0.25]
+ |   - [-0.125]
+ |   - [0]
+ |   - [0.125]
+ |   - [0.25]
+ |   - [0.375]
+ |   - [0.5]
+ |   - [0.625]
+ |   - [0.75]
+ |   - [0.875]
+ |   - [1]
+ |   - [1.125]
+ |   - [1.25]
+ |   - [1.375]
+ |   - [1.5]
+ |   - [1.625]
+ |   - [1.75]
+ |   - [1.875]
+ |   - [2]
+ | ...
+
+-- check duplicates
+box.space.test:insert{-2}
+ | ---
+ | - error: Duplicate key exists in unique index 'pk' in space 'test'
+ | ...
+box.space.test:insert{decimal.new(-2)}
+ | ---
+ | - error: Duplicate key exists in unique index 'pk' in space 'test'
+ | ...
+box.space.test:insert{decimal.new(-1.875)}
+ | ---
+ | - error: Duplicate key exists in unique index 'pk' in space 'test'
+ | ...
+box.space.test:insert{-1.875}
+ | ---
+ | - error: Duplicate key exists in unique index 'pk' in space 'test'
+ | ...
+
+box.space.test.index.pk:drop()
+ | ---
+ | ...
+
+_ = box.space.test:create_index('pk')
+ | ---
+ | ...
+test_run:cmd('setopt delimiter ";"')
+ | ---
+ | - true
+ | ...
+for i = 1,10 do
+    box.space.test:insert{i, decimal.new(i/10)}
+end;
+ | ---
+ | ...
+test_run:cmd('setopt delimiter ""');
+ | ---
+ | - true
+ | ...
+
+-- a bigger test with a secondary index this time.
+box.space.test:insert{11, 'str'}
+ | ---
+ | - [11, 'str']
+ | ...
+box.space.test:insert{12, 0.63}
+ | ---
+ | - [12, 0.63]
+ | ...
+box.space.test:insert{13, 0.57}
+ | ---
+ | - [13, 0.57]
+ | ...
+box.space.test:insert{14, 0.33}
+ | ---
+ | - [14, 0.33]
+ | ...
+box.space.test:insert{16, 0.71}
+ | ---
+ | - [16, 0.71]
+ | ...
+
+_ = box.space.test:create_index('sk', {parts={2, 'scalar'}})
+ | ---
+ | ...
+box.space.test.index.sk:select{}
+ | ---
+ | - - [1, 0.1]
+ |   - [2, 0.2]
+ |   - [3, 0.3]
+ |   - [14, 0.33]
+ |   - [4, 0.4]
+ |   - [5, 0.5]
+ |   - [13, 0.57]
+ |   - [6, 0.6]
+ |   - [12, 0.63]
+ |   - [7, 0.7]
+ |   - [16, 0.71]
+ |   - [8, 0.8]
+ |   - [9, 0.9]
+ |   - [10, 1]
+ |   - [11, 'str']
+ | ...
+
+box.space.test:drop()
+ | ---
+ | ...
diff --git a/test/engine/decimal.test.lua b/test/engine/decimal.test.lua
new file mode 100644
index 000000000..52a300e72
--- /dev/null
+++ b/test/engine/decimal.test.lua
@@ -0,0 +1,65 @@
+env = require('test_run')
+test_run = env.new()
+engine = test_run:get_cfg('engine')
+
+decimal = require('decimal')
+ffi = require('ffi')
+
+_ = box.schema.space.create('test', {engine=engine})
+_ = box.space.test:create_index('pk', {parts={1,'decimal'}})
+
+test_run:cmd('setopt delimiter ";"')
+for i = 0,16 do
+    box.space.test:insert{decimal.new((i-8)/4)}
+end;
+test_run:cmd('setopt delimiter ""');
+
+box.space.test:select{}
+
+-- check invalid values
+box.space.test:insert{1.23}
+box.space.test:insert{'str'}
+box.space.test:insert{ffi.new('uint64_t', 0)}
+-- check duplicates
+box.space.test:insert{decimal.new(0)}
+
+box.space.test.index.pk:drop()
+
+_ = box.space.test:create_index('pk', {parts={1, 'number'}})
+
+test_run:cmd('setopt delimiter ";"')
+for i = 0, 32 do
+    local val = (i - 16) / 8
+    if i % 2 == 1 then val = decimal.new(val) end
+    box.space.test:insert{val}
+end;
+test_run:cmd('setopt delimiter ""');
+
+box.space.test:select{}
+
+-- check duplicates
+box.space.test:insert{-2}
+box.space.test:insert{decimal.new(-2)}
+box.space.test:insert{decimal.new(-1.875)}
+box.space.test:insert{-1.875}
+
+box.space.test.index.pk:drop()
+
+_ = box.space.test:create_index('pk')
+test_run:cmd('setopt delimiter ";"')
+for i = 1,10 do
+    box.space.test:insert{i, decimal.new(i/10)}
+end;
+test_run:cmd('setopt delimiter ""');
+
+-- a bigger test with a secondary index this time.
+box.space.test:insert{11, 'str'}
+box.space.test:insert{12, 0.63}
+box.space.test:insert{13, 0.57}
+box.space.test:insert{14, 0.33}
+box.space.test:insert{16, 0.71}
+
+_ = box.space.test:create_index('sk', {parts={2, 'scalar'}})
+box.space.test.index.sk:select{}
+
+box.space.test:drop()
-- 
2.20.1 (Apple Git-117)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/5] decimal: fix decimal.round() when scale == 0
  2019-07-17 15:33 ` [PATCH 1/5] decimal: fix decimal.round() when scale == 0 Serge Petrenko
@ 2019-07-24 12:10   ` Vladimir Davydov
  2019-07-24 23:02   ` [tarantool-patches] " Konstantin Osipov
  1 sibling, 0 replies; 16+ messages in thread
From: Vladimir Davydov @ 2019-07-24 12:10 UTC (permalink / raw)
  To: Serge Petrenko; +Cc: tarantool-patches

On Wed, Jul 17, 2019 at 06:33:42PM +0300, Serge Petrenko wrote:
> Fixes decimal.round() with zero scale, fixes an error with
> decimal.round() when rounding leads to a number with the same
> precision, for example, decimal.round(9.9, 0) -> 10.
> 
> Follow-up #692
> ---
>  src/lib/core/decimal.c    |  8 ++++----
>  test/app/decimal.result   | 18 ++++++++++++++++++
>  test/app/decimal.test.lua |  6 ++++++
>  3 files changed, 28 insertions(+), 4 deletions(-)

Pushed to master.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/5] lua: allow to encode and decode decimals as msgpack
  2019-07-17 15:33 ` [PATCH 3/5] lua: allow to encode and decode decimals as msgpack Serge Petrenko
@ 2019-07-24 14:11   ` Vladimir Davydov
  2019-07-24 23:10   ` [tarantool-patches] " Konstantin Osipov
  1 sibling, 0 replies; 16+ messages in thread
From: Vladimir Davydov @ 2019-07-24 14:11 UTC (permalink / raw)
  To: Serge Petrenko; +Cc: tarantool-patches

On Wed, Jul 17, 2019 at 06:33:44PM +0300, Serge Petrenko wrote:
> diff --git a/src/lib/core/decimal.c b/src/lib/core/decimal.c
> index 1423ae418..2c69a773c 100644
> --- a/src/lib/core/decimal.c
> +++ b/src/lib/core/decimal.c
> @@ -33,6 +33,7 @@
>  #include "third_party/decNumber/decContext.h"
>  #include "third_party/decNumber/decPacked.h"
>  #include "lib/core/tt_static.h"
> +#include "lib/msgpuck/msgpuck.h"
>  #include <stddef.h>
>  #include <stdlib.h>
>  #include <float.h> /* DBL_DIG */
> @@ -312,12 +313,15 @@ char *
>  decimal_pack(char *data, const decimal_t *dec)
>  {
>  	uint32_t len = decimal_len(dec);
> -	*data++ = decimal_scale(dec);
> +	/* reserve space for resulting scale */
> +	char *svp = data++;
>  	len--;
>  	int32_t scale;
>  	char *tmp = (char *)decPackedFromNumber((uint8_t *)data, len, &scale, dec);
>  	assert(tmp == data);
> -	assert(scale == (int32_t)decimal_scale(dec));
> +	/* scale may be negative, when exponent is > 0 */
> +	assert(scale == (int32_t)decimal_scale(dec) || scale < 0);
> +	mp_store_u8(svp, (int8_t)scale);
>  	(void)tmp;
>  	data += len;
>  	return data;
> @@ -326,7 +330,7 @@ decimal_pack(char *data, const decimal_t *dec)
>  decimal_t *
>  decimal_unpack(const char **data, uint32_t len, decimal_t *dec)
>  {
> -	int32_t scale = *((*data)++);
> +	int32_t scale = (int8_t)mp_load_u8(data);
>  	len--;
>  	decimal_t *res = decPackedToNumber((uint8_t *)*data, len, &scale, dec);
>  	if (res)

How is this related to this patch? Is this a fix? If so, it should be
submitted in a separate patch with an appropriate test case.

> diff --git a/src/lib/core/mp_user_types.h b/src/lib/core/mp_user_types.h
> index 9158b40d3..8211e3e79 100644
> --- a/src/lib/core/mp_user_types.h
> +++ b/src/lib/core/mp_user_types.h
> @@ -32,7 +32,8 @@
>   */
>  
>  enum mp_user_type {
> -    MP_DECIMAL = 0
> +    MP_UNKNOWN = 0,
> +    MP_DECIMAL = 1

MP_UNKNOWN doesn't make any sense IMO. AFAICS you need it solely to
convert Lua to msgpack and back. Let's please try to get along without
it. See also my comment to luamp_encode_r.

>  };
>  
>  #endif
> diff --git a/src/lib/core/mpstream.c b/src/lib/core/mpstream.c
> index 8b7276ab1..a46b7962b 100644
> --- a/src/lib/core/mpstream.c
> +++ b/src/lib/core/mpstream.c
> @@ -33,6 +33,7 @@
>  #include <assert.h>
>  #include <stdint.h>
>  #include "msgpuck.h"
> +#include "mp_decimal.h"
>  
>  void
>  mpstream_reserve_slow(struct mpstream *stream, size_t size)
> @@ -186,6 +187,16 @@ mpstream_encode_binl(struct mpstream *stream, uint32_t len)
>  	mpstream_advance(stream, pos - data);
>  }
>  
> +void
> +mpstream_encode_decimal(struct mpstream *stream, decimal_t *val)

val should be const

> +{
> +	char *data = mpstream_reserve(stream, mp_sizeof_decimal(val));
> +	if (data == NULL)
> +		return;
> +	char *pos = mp_encode_decimal(data, val);
> +	mpstream_advance(stream, pos - data);
> +}
> +
>  void
>  mpstream_memcpy(struct mpstream *stream, const void *src, uint32_t n)
>  {
> diff --git a/src/lua/decimal.c b/src/lua/decimal.c
> index e548cdb9d..ab8d85f75 100644
> --- a/src/lua/decimal.c
> +++ b/src/lua/decimal.c
> @@ -31,7 +31,7 @@
>  
>  #include "lua/decimal.h"
>  #include "lib/core/decimal.h"
> -#include "lua/utils.h"
> +#include "lua/utils.h" /* CTID_DECIMAL, ... */

Why not define CTID in lua/decimal.h?

>  
>  #include <lua.h>
>  #include <lauxlib.h>
> @@ -69,16 +69,18 @@ ldecimal_##name(struct lua_State *L) {						\
>  static int									\
>  ldecimal_##name(struct lua_State *L) {						\
>  	assert(lua_gettop(L) == 2);						\
> +	if (lua_isnil(L, 1) || lua_isnil(L, 2)) {				\
> +		lua_pushboolean(L, false);					\
> +		return 1;							\
> +	}									\

What is this change for?

> diff --git a/src/lua/msgpack.c b/src/lua/msgpack.c
> index 2126988eb..3cb7d7dcd 100644
> --- a/src/lua/msgpack.c
> +++ b/src/lua/msgpack.c
> @@ -41,6 +41,10 @@
>  #include <small/region.h>
>  #include <small/ibuf.h>
>  
> +#include "lua/decimal.h"
> +#include "lib/core/decimal.h"
> +#include "lib/core/mp_user_types.h"
> +
>  #include <fiber.h>
>  
>  void
> @@ -175,16 +179,23 @@ restart: /* used by MP_EXT */
>  		assert(lua_gettop(L) == top);
>  		return MP_ARRAY;
>  	case MP_EXT:
> -		/* Run trigger if type can't be encoded */
> -		type = luamp_encode_extension(L, top, stream);
> -		if (type != MP_EXT)
> -			return type; /* Value has been packed by the trigger */
> -		/* Try to convert value to serializable type */
> -		luaL_convertfield(L, cfg, top, field);
> -		/* handled by luaL_convertfield */
> -		assert(field->type != MP_EXT);
> -		assert(lua_gettop(L) == top);
> -		goto restart;
> +		switch (field->ext_type)
> +		{
> +		case MP_DECIMAL:
> +			mpstream_encode_decimal(stream, field->decval);
> +			return MP_EXT;
> +		case MP_UNKNOWN:
> +			/* Run trigger if type can't be encoded */
> +			type = luamp_encode_extension(L, top, stream);
> +			if (type != MP_EXT)
> +				return type; /* Value has been packed by the trigger */
> +			/* Try to convert value to serializable type */
> +			luaL_convertfield(L, cfg, top, field);
> +			/* handled by luaL_convertfield */
> +			assert(field->type != MP_EXT || field->ext_type != MP_UNKNOWN);
> +			assert(lua_gettop(L) == top);
> +			goto restart;
> +		}

I don't like this two-level switch-case. AFAIU we don't really need to
use MP_* types for luaL_field::type. We could instead introduce a new
enum (like LUA_FIELD_*). This would allow us to make this switch-case
plane. Please consider it. This could be done in a separate patch.

>  	}
>  	return MP_EXT;
>  }
> @@ -283,9 +294,28 @@ luamp_decode(struct lua_State *L, struct luaL_serializer *cfg,
>  		return;
>  	}
>  	case MP_EXT:
> -		luamp_decode_extension(L, data);
> +	{
> +		uint32_t len;
> +		int8_t type;
> +		len = mp_decode_extl(data, &type);
> +		switch (type) {
> +		case MP_DECIMAL:
> +		{
> +			decimal_t *dec = lua_pushdecimal(L);
> +			dec = decimal_unpack(data, len, dec);
> +			if (dec == NULL) {
> +				lua_pop(L, -1);
> +				luaL_error(L, "msgpack.decode: "
> +					      "invalid MsgPack.");
> +			}
> +			return;
> +		}
> +		default:
> +			luamp_decode_extension(L, data);
> +		}
>  		break;
>  	}
> +	}

Again, two-level switch-case doesn't look good. In this case I think we
could add a helper function which would determine LUA_FIELD_* type by
looking at data.

>  }
>  
>  
> diff --git a/src/lua/msgpackffi.lua b/src/lua/msgpackffi.lua
> index bfeedbc4b..4c799eed0 100644
> --- a/src/lua/msgpackffi.lua
> +++ b/src/lua/msgpackffi.lua
> @@ -17,10 +17,18 @@ char *
>  mp_encode_float(char *data, float num);
>  char *
>  mp_encode_double(char *data, double num);
> +char *
> +mp_encode_decimal(char *data, decimal_t *dec);
> +uint32_t
> +mp_sizeof_decimal(const decimal_t *dec);
>  float
>  mp_decode_float(const char **data);
>  double
>  mp_decode_double(const char **data);
> +uint32_t
> +mp_decode_extl(const char **data, int8_t *type);
> +decimal_t *
> +decimal_unpack(const char **data, uint32_t len, decimal_t *dec);
>  ]])
>  
>  local strict_alignment = (jit.arch == 'arm')
> @@ -117,6 +125,11 @@ local function encode_double(buf, num)
>      builtin.mp_encode_double(p, num)
>  end
>  
> +local function encode_decimal(buf, num)
> +    local p = buf:alloc(builtin.mp_sizeof_decimal(num))
> +    builtin.mp_encode_decimal(p, num)
> +end
> +
>  local function encode_int(buf, num)
>      if num >= 0 then
>          if num <= 0x7f then
> @@ -294,6 +307,7 @@ on_encode(ffi.typeof('const unsigned char'), encode_int)
>  on_encode(ffi.typeof('bool'), encode_bool_cdata)
>  on_encode(ffi.typeof('float'), encode_float)
>  on_encode(ffi.typeof('double'), encode_double)
> +on_encode(ffi.typeof('decimal_t'), encode_decimal)
>  
>  --------------------------------------------------------------------------------
>  -- Decoder
> @@ -473,6 +487,23 @@ local function decode_map(data, size)
>      return setmetatable(map, msgpack.map_mt)
>  end
>  
> +local function decode_ext(data)
> +    local t = ffi.new("int8_t[1]")
> +    -- mp_decode_extl and mp_decode_decimal
> +    -- need type code
> +    data[0] = data[0] - 1
> +    local old_data = data[0]
> +    local len = builtin.mp_decode_extl(data, t)
> +    --MP_DECIMAL
> +    if t[0] == 1 then
> +        local num = ffi.new("decimal_t")
> +        builtin.decimal_unpack(data, len, num)
> +        return num
> +    else
> +        error("Unsupported extension type")
> +    end
> +end
> +
>  local decoder_hint = {
>      --[[{{{ MP_BIN]]
>      [0xc4] = function(data) return decode_str(data, decode_u8(data)) end;
> @@ -528,6 +559,8 @@ decode_r = function(data)
>          return false
>      elseif c == 0xc3 then
>          return true
> +    elseif c >= 0xd4 and c <= 0xd8 or c >= 0xc7 and c <= 0xc9 then
> +        return decode_ext(data)

Could you please introduce some infrastructure that would allow us to
easily add new MP_EXT types in future? Some kind of conversion table
mapping ext type to a decoder function, may be?

> diff --git a/src/lua/utils.h b/src/lua/utils.h
> index 7e7cdc0c6..3ca43292b 100644
> --- a/src/lua/utils.h
> +++ b/src/lua/utils.h
> @@ -54,6 +54,8 @@ extern "C" {
>  #include <lj_meta.h>
>  
>  #include "lua/error.h"
> +#include "lib/core/mp_user_types.h"

This inclusion seems to be pointless.

> diff --git a/test/app/msgpack.test.lua b/test/app/msgpack.test.lua
> index e0880ac22..9224d870a 100644
> --- a/test/app/msgpack.test.lua
> +++ b/test/app/msgpack.test.lua
> @@ -84,3 +84,18 @@ size = msgpack.encode(100, buf)
>  -- is not negative.
>  --
>  msgpack.decode(ffi.cast('char *', '\x04\x05\x06'), -1)
> +
> +--
> +-- gh-4333: msgpack encode/decode decimals.
> +--
> +decimal = require('decimal')
> +a = decimal.new('1e37')
> +b = decimal.new('1e-38')
> +c = decimal.new('1')
> +d = decimal.new('0.1234567')
> +e = decimal.new('123.4567')
> +msgpack.decode(msgpack.encode(a)) == a
> +msgpack.decode(msgpack.encode(b)) == b
> +msgpack.decode(msgpack.encode(c)) == c
> +msgpack.decode(msgpack.encode(d)) == d
> +msgpack.decode(msgpack.encode(e)) == e

Please also test

 - msgpackffi
 - storing and accessing decimal in tuples
 - storing decimals in spaces

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] decimal: add conversions to (u)int64_t
  2019-07-17 15:33 ` [PATCH 4/5] decimal: add conversions to (u)int64_t Serge Petrenko
@ 2019-07-24 16:17   ` Vladimir Davydov
  0 siblings, 0 replies; 16+ messages in thread
From: Vladimir Davydov @ 2019-07-24 16:17 UTC (permalink / raw)
  To: Serge Petrenko; +Cc: tarantool-patches

On Wed, Jul 17, 2019 at 06:33:45PM +0300, Serge Petrenko wrote:
> Update decNumber library, add methods to convert decimals to uint64_t
> and int64_t, add unit tests.
> Also replace decimal_round() function with decimal_round_with_mode() to
> allow setting rounding mode.

Better add decimal_floor(), as decimal_round_with_mode() has a
complicated API IMO.

> We need to round with mode DEC_ROUND_DOWN in to_int64 conversions in
> order to be consistent with double to int conversions.
> It will be needed to compute hints for decimal fields.
> 
> Prerequisite #4333
> ---
>  src/lib/core/decimal.c   | 63 ++++++++++++++++++++++++++++++++--
>  src/lib/core/decimal.h   | 12 +++++++
>  test/unit/decimal.c      | 66 ++++++++++++++++++++++++++++++++++-
>  test/unit/decimal.result | 74 ++++++++++++++++++++++++++++++++++++++--
>  third_party/decNumber    |  2 +-
>  5 files changed, 209 insertions(+), 8 deletions(-)
> 
> diff --git a/src/lib/core/decimal.c b/src/lib/core/decimal.c
> index 2c69a773c..5a576e6e5 100644
> --- a/src/lib/core/decimal.c
> +++ b/src/lib/core/decimal.c
> @@ -157,6 +157,57 @@ decimal_to_string(const decimal_t *dec)
>  	return buf;
>  }
>  
> +static decimal_t *
> +decimal_round_with_mode(decimal_t *dec, int scale, enum rounding mode);
> +
> +decimal_t *
> +decimal_to_int64(decimal_t *dec, int64_t *num)

Why return decimal_t?

Shouldn't 'dec' be marked const?

> +{
> +	decimal_t d, z;
> +	decNumberZero(&z);
> +	d = *dec;
> +	dec = &d;

This looks confusing - like you're modifying the given decimal
number below.

> +
> +	if (decimal_scale(dec) != 0) {
> +		/*
> +		 * Rounding mode is important here.
> +		 * We want to be consistent with double
> +		 * to int conversion so that comparison
> +		 * hints work correctly.
> +		 */
> +		dec = decimal_round_with_mode(dec, 0, DEC_ROUND_DOWN);
> +	}
> +	/*
> +	 * decNumberToInt64 works only with numbers with
> +	 * zero exponent.
> +	 */
> +	decNumberRescale(dec, dec, &z, &decimal_context);
> +	if (decimal_check_status(dec, &decimal_context) == NULL) {
> +		return NULL;
> +	}
> +	*num = decNumberToInt64(dec, &decimal_context);
> +	return decimal_check_status(dec, &decimal_context);
> +}
> +
> +decimal_t *
> +decimal_to_uint64(decimal_t *dec, uint64_t *num)
> +{
> +	decimal_t d, z;
> +	decNumberZero(&z);
> +	d = *dec;
> +	dec = &d;
> +
> +	if (decimal_scale(dec) != 0) {
> +		dec = decimal_round_with_mode(dec, 0, DEC_ROUND_DOWN);
> +	}
> +	decNumberRescale(dec, dec, &z, &decimal_context);

It would be nice to factor out the piece of code shared by
decimal_to_uint64 and decimal_to_int64.

> +	if (decimal_check_status(dec, &decimal_context) == NULL) {
> +		return NULL;
> +	}
> +	*num = decNumberToUInt64(dec, &decimal_context);
> +	return decimal_check_status(dec, &decimal_context);
> +}
> +

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5] decimal: allow to encode/decode decimals as MsgPack
  2019-07-17 15:33 ` [PATCH 2/5] decimal: allow to encode/decode decimals as MsgPack Serge Petrenko
@ 2019-07-24 16:19   ` Vladimir Davydov
  2019-07-24 23:08   ` [tarantool-patches] " Konstantin Osipov
  1 sibling, 0 replies; 16+ messages in thread
From: Vladimir Davydov @ 2019-07-24 16:19 UTC (permalink / raw)
  To: Serge Petrenko; +Cc: tarantool-patches

This one looks good to me.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/5] decimal: allow to index decimals
  2019-07-17 15:33 ` [PATCH 5/5] decimal: allow to index decimals Serge Petrenko
@ 2019-07-24 16:56   ` Vladimir Davydov
  2019-07-24 23:13   ` [tarantool-patches] " Konstantin Osipov
  2019-07-24 23:17   ` Konstantin Osipov
  2 siblings, 0 replies; 16+ messages in thread
From: Vladimir Davydov @ 2019-07-24 16:56 UTC (permalink / raw)
  To: Serge Petrenko; +Cc: tarantool-patches

On Wed, Jul 17, 2019 at 06:33:46PM +0300, Serge Petrenko wrote:
> Indices can now be built over decimal fields.
> A new field type - 'decimal' is introduced.
> Decimal values may be stored in 'decimal' columns, as well as in
> 'scalar' and 'any' columns.

You can remove this text as it reiterates the doc bot request below.

> 
> Closes #4333
> 
> @TarantoolBot document
> Title: Document decimal field type.
> 
> Decimals may now be stored in spaces. A corresponding field type is
> introduced: 'decimal'. Decimal values are also allowed in 'scalar' and
> 'number' fields.
> 
> 'decimal' field type is appropriate for both memtx HASH and TREE
> indices, as well as for vinyl TREE index.

Please describe how decimals are ordered in respect to other numeric
types. Give an example of creating and using an index over a decimal
field.

Can one use decimal in space format? If so, please mention it too
and add some tests.

What about update operations (index.update, tuple.update). Do they
support decimals? I assume not for now. This should be addressed
(a separate patch is okay).

> ---
>  src/box/field_def.c          |  43 +++++--
>  src/box/field_def.h          |  16 ++-
>  src/box/key_def.h            |   2 +-
>  src/box/tuple_compare.cc     | 160 ++++++++++++++++++++++++-
>  src/box/tuple_format.c       |   2 +-
>  src/lib/core/decimal.h       |   8 ++
>  src/lib/core/mp_decimal.h    |   8 ++
>  test/engine/decimal.result   | 226 +++++++++++++++++++++++++++++++++++
>  test/engine/decimal.test.lua |  65 ++++++++++
>  9 files changed, 510 insertions(+), 20 deletions(-)
>  create mode 100644 test/engine/decimal.result
>  create mode 100644 test/engine/decimal.test.lua
> 
> diff --git a/src/box/field_def.c b/src/box/field_def.c
> index 346042b98..da06e6bde 100644
> --- a/src/box/field_def.c
> +++ b/src/box/field_def.c
> @@ -52,17 +52,32 @@ const uint32_t field_mp_type[] = {
>  	/* [FIELD_TYPE_UNSIGNED] =  */ 1U << MP_UINT,
>  	/* [FIELD_TYPE_STRING]   =  */ 1U << MP_STR,
>  	/* [FIELD_TYPE_NUMBER]   =  */ (1U << MP_UINT) | (1U << MP_INT) |
> -		(1U << MP_FLOAT) | (1U << MP_DOUBLE),
> +		(1U << MP_FLOAT) | (1U << MP_DOUBLE) | (1U << MP_EXT),
>  	/* [FIELD_TYPE_INTEGER]  =  */ (1U << MP_UINT) | (1U << MP_INT),
>  	/* [FIELD_TYPE_BOOLEAN]  =  */ 1U << MP_BOOL,
>  	/* [FIELD_TYPE_VARBINARY] =  */ 1U << MP_BIN,
>  	/* [FIELD_TYPE_SCALAR]   =  */ (1U << MP_UINT) | (1U << MP_INT) |
>  		(1U << MP_FLOAT) | (1U << MP_DOUBLE) | (1U << MP_STR) |
> -		(1U << MP_BIN) | (1U << MP_BOOL),
> +		(1U << MP_BIN) | (1U << MP_BOOL) | (1U << MP_EXT),
> +	/* [FIELD_TYPE_DECIMAL]  =  */ 1U << MP_EXT,
>  	/* [FIELD_TYPE_ARRAY]    =  */ 1U << MP_ARRAY,
>  	/* [FIELD_TYPE_MAP]      =  */ (1U << MP_MAP),
>  };
>  
> +const uint32_t field_ext_type[] = {

field_mp_ext_type

> +	/* [FIELD_TYPE_ANY]      =  */ UINT32_MAX & ~(1U << MP_UNKNOWN),

Can one insert MP_EXT other than decimal into a space? Do we need to
support that?

> +	/* [FIELD_TYPE_UNSIGNED] =  */ 0,
> +	/* [FIELD_TYPE_STRING]   =  */ 0,
> +	/* [FIELD_TYPE_NUMBER]   =  */ 1U << MP_DECIMAL,
> +	/* [FIELD_TYPE_INTEGER]  =  */ 0,
> +	/* [FIELD_TYPE_BOOLEAN]  =  */ 0,
> +	/* [FIELD_TYPE_VARBINARY] = */ 0,
> +	/* [FIELD_TYPE_SCALAR]   =  */ 1U << MP_DECIMAL,
> +	/* [FIELD_TYPE_DECIMAL]  =  */ 1U << MP_DECIMAL,
> +	/* [FIELD_TYPE_ARRAY]    =  */ 0,
> +	/* [FIELD_TYPE_MAP]      =  */ 0,
> +};
> +
> diff --git a/src/box/key_def.h b/src/box/key_def.h
> index f4a1a8fd1..ed3354ade 100644
> --- a/src/box/key_def.h
> +++ b/src/box/key_def.h
> @@ -505,7 +505,7 @@ static inline int
>  key_part_validate(enum field_type key_type, const char *key,
>  		  uint32_t field_no, bool is_nullable)
>  {
> -	if (unlikely(!field_mp_type_is_compatible(key_type, mp_typeof(*key),
> +	if (unlikely(!field_mp_type_is_compatible(key_type, key,
>  						  is_nullable))) {
>  		diag_set(ClientError, ER_KEY_PART_TYPE, field_no,
>  			 field_type_strs[key_type]);
> diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc
> index 95a0f58c9..6ab28f662 100644
> --- a/src/box/tuple_compare.cc
> +++ b/src/box/tuple_compare.cc
> @@ -33,6 +33,9 @@
>  #include "coll/coll.h"
>  #include "trivia/util.h" /* NOINLINE */
>  #include <math.h>
> +#include "lib/core/decimal.h"
> +#include "lib/core/mp_decimal.h"
> +#include "lib/core/mp_user_types.h"
>  
>  /* {{{ tuple_compare */
>  
> @@ -87,7 +90,7 @@ static enum mp_class mp_classes[] = {
>  	/* .MP_BOOL    = */ MP_CLASS_BOOL,
>  	/* .MP_FLOAT   = */ MP_CLASS_NUMBER,
>  	/* .MP_DOUBLE  = */ MP_CLASS_NUMBER,
> -	/* .MP_BIN     = */ MP_CLASS_BIN
> +	/* .MP_EXT     = */ MP_CLASS_NUMBER /* requires additional parsing */

This, as well as the use of MP_EXT below, looks confusing: you imply
that MP_EXT is always a number, but what happens when we add TIMESTAMP
or UUID ext type?

I would instead add a new enum for all comparable types (bool, float,
decimal, etc) and a helper that would determine the type by msgpack
data. This way all switch-cases below would be flat and the code would
be generally easier for understanding IMO. Please consider this. This
could be done in a separate patch.

>  };
>  
>  #define COMPARE_RESULT(a, b) (a < b ? -1 : a > b)
> @@ -264,6 +267,50 @@ mp_compare_double_any_number(double lhs, const char *rhs,
>  	return k * COMPARE_RESULT(lqbit, rqbit);
>  }
>  
> +static int
> +mp_compare_decimal_any_number(decimal_t *lhs, const char *rhs,
> +			      enum mp_type rhs_type, int k)
> +{
> +	decimal_t rhs_dec;
> +	switch (rhs_type) {
> +	case MP_FLOAT:
> +	{
> +		double d = mp_decode_float(&rhs);
> +		decimal_from_double(&rhs_dec, d);
> +		break;
> +	}
> +	case MP_DOUBLE:
> +	{
> +		double d = mp_decode_double(&rhs);
> +		decimal_from_double(&rhs_dec, d);
> +		break;
> +	}
> +	case MP_INT:
> +	{
> +		int64_t num = mp_decode_int(&rhs);
> +		decimal_from_int64(&rhs_dec, num);
> +		break;
> +	}
> +	case MP_UINT:
> +	{
> +		uint64_t num = mp_decode_uint(&rhs);
> +		decimal_from_uint64(&rhs_dec, num);
> +		break;
> +	}
> +	case MP_EXT:
> +	{
> +		int8_t ext_type;
> +		uint32_t len = mp_decode_extl(&rhs, &ext_type);
> +		assert(ext_type == MP_DECIMAL);
> +		decimal_unpack(&rhs, len, &rhs_dec);
> +		break;
> +	}
> +	default:
> +		unreachable();
> +	}
> +	return k * decimal_compare(lhs, &rhs_dec);
> +}
> +
>  static int
>  mp_compare_number_with_type(const char *lhs, enum mp_type lhs_type,
>  			    const char *rhs, enum mp_type rhs_type)
> @@ -271,6 +318,21 @@ mp_compare_number_with_type(const char *lhs, enum mp_type lhs_type,
>  	assert(mp_classof(lhs_type) == MP_CLASS_NUMBER);
>  	assert(mp_classof(rhs_type) == MP_CLASS_NUMBER);
>  
> +	/*
> +	 * test decimals first, so that we don't have to
> +	 * account for them in other comarators.
> +	 */
> +	decimal_t dec;
> +	if (rhs_type == MP_EXT) {
> +		return mp_compare_decimal_any_number(
> +			mp_decode_decimal(&rhs, &dec), lhs, lhs_type, -1
> +		);
> +	}
> +	if (lhs_type == MP_EXT) {
> +		return mp_compare_decimal_any_number(
> +			mp_decode_decimal(&lhs, &dec), rhs, rhs_type, 1
> +		);
> +	}
>  	if (rhs_type == MP_FLOAT) {
>  		return mp_compare_double_any_number(
>  			mp_decode_float(&rhs), lhs, lhs_type, -1
> @@ -410,6 +472,8 @@ tuple_compare_field(const char *field_a, const char *field_b,
>  		return coll != NULL ?
>  		       mp_compare_scalar_coll(field_a, field_b, coll) :
>  		       mp_compare_scalar(field_a, field_b);
> +	case FIELD_TYPE_DECIMAL:
> +		return mp_compare_number(field_a, field_b);
>  	default:
>  		unreachable();
>  		return 0;
> @@ -443,6 +507,8 @@ tuple_compare_field_with_type(const char *field_a, enum mp_type a_type,
>  		       mp_compare_scalar_coll(field_a, field_b, coll) :
>  		       mp_compare_scalar_with_type(field_a, a_type,
>  						   field_b, b_type);
> +	case FIELD_TYPE_DECIMAL:
> +		return mp_compare_number(field_a, field_b);
>  	default:
>  		unreachable();
>  		return 0;
> @@ -1356,6 +1422,25 @@ static const comparator_with_key_signature cmp_wk_arr[] = {
>  #define HINT_VALUE_DOUBLE_MAX	(exp2(HINT_VALUE_BITS - 1) - 1)
>  #define HINT_VALUE_DOUBLE_MIN	(-exp2(HINT_VALUE_BITS - 1))
>  
> +/**
> + * Max and min decimal numbers whose integral parts fit
> + * in a hint value.
> + */
> +static const decimal_t HINT_VALUE_DECIMAL_MAX = {
> +	18,				/* decimal digits */
> +	0,				/* exponent */
> +	0,				/* no special bits. */
> +	{487, 423, 303, 752, 460, 576}	/* 576,460,752,303,423,488 = 2^59 - 1 */
> +					/* 59 == HINT_VALUE_BITS - 1 */
> +};
> +
> +static const decimal_t HINT_VALUE_DECIMAL_MIN = {
> +	18,				/* decimal digits */
> +	0,				/* exponent */
> +	0x80,				/* negative bit */
> +	{488, 423, 303, 752, 460, 576}	/* 576,460,752,303,423,488 = 2^59 */
> +};
> +

This looks fragile. Suppose we remove one bit from a hint, how should
one update those? Please define these constants using decimal_from_int64
or, even better, get rid of them altogether (see below).

>  /*
>   * HINT_CLASS_BITS should be big enough to store any mp_class value.
>   * Note, ((1 << HINT_CLASS_BITS) - 1) is reserved for HINT_NONE.
> @@ -1415,6 +1500,25 @@ hint_double(double d)
>  	return hint_create(MP_CLASS_NUMBER, val);
>  }
>  
> +static inline hint_t
> +hint_decimal(decimal_t *dec)
> +{
> +	uint64_t val = 0;
> +	int64_t num;
> +	if (decimal_compare(dec, &HINT_VALUE_DECIMAL_MAX) >= 0)
> +		val = HINT_VALUE_MAX;
> +	else if (decimal_compare(dec, &HINT_VALUE_DECIMAL_MIN) <= 0)
> +		val = 0;
> +	else {
> +		dec = decimal_to_int64(dec, &num);
> +		/* We've checked boundaries above. */
> +		assert(dec != NULL);
> +		val = num - HINT_VALUE_INT_MIN;
> +	}

Why not call decimal_to_int64 first and then calculate a hint from the
returned number? I mean

  if (decimal_to_int64(dec, &num) &&
      num >= HINT_VALUE_INT_MIN && num <= HINT_VALUE_INT_MAX)
          hint = num;
  else
          ...

> +
> +	return hint_create(MP_CLASS_NUMBER, val);
> +}
> +
>  static inline uint64_t
>  hint_str_raw(const char *s, uint32_t len)
>  {
> @@ -1491,12 +1595,42 @@ field_hint_number(const char *field)
>  		return hint_double(mp_decode_float(&field));
>  	case MP_DOUBLE:
>  		return hint_double(mp_decode_double(&field));
> +	case MP_EXT:
> +	{
> +		int8_t ext_type;
> +		uint32_t len = mp_decode_extl(&field, &ext_type);
> +		switch(ext_type) {
> +		case MP_DECIMAL:
> +		{
> +			decimal_t dec;
> +			decimal_t *res;
> +			/*
> +			 * The effect of mp_decode_extl() +
> +			 * decimal_unpack() is the same that
> +			 * the one of mp_decode_decimal().
> +			 */
> +			res = decimal_unpack(&field, len, &dec);
> +			assert(res != NULL);
> +			return hint_decimal(res);
> +		}
> +		default:
> +			unreachable();
> +		}
> +	}
>  	default:
>  		unreachable();
>  	}
>  	return HINT_NONE;
>  }
>  
> +static inline hint_t
> +field_hint_decimal(const char *field)
> +{
> +	assert(mp_typeof(*field) == MP_EXT);
> +	decimal_t dec;
> +	return hint_decimal(mp_decode_decimal(&field, &dec));
> +}
> +
>  static inline hint_t
>  field_hint_string(const char *field, struct coll *coll)
>  {
> @@ -1536,6 +1670,25 @@ field_hint_scalar(const char *field, struct coll *coll)
>  	case MP_BIN:
>  		len = mp_decode_binl(&field);
>  		return hint_bin(field, len);
> +	case MP_EXT:
> +	{
> +		int8_t ext_type;
> +		uint32_t len = mp_decode_extl(&field, &ext_type);
> +		switch(ext_type) {
> +		case MP_DECIMAL:
> +		{
> +			decimal_t dec;
> +			/*
> +			 * The effect of mp_decode_extl() +
> +			 * decimal_unpack() is the same that
> +			 * the one of mp_decode_decimal().
> +			 */
> +			return hint_decimal(decimal_unpack(&field, len, &dec));
> +		}

Please try to eliminate code duplication.

> diff --git a/test/engine/decimal.test.lua b/test/engine/decimal.test.lua
> new file mode 100644
> index 000000000..52a300e72
> --- /dev/null
> +++ b/test/engine/decimal.test.lua
> @@ -0,0 +1,65 @@
> +env = require('test_run')
> +test_run = env.new()
> +engine = test_run:get_cfg('engine')
> +
> +decimal = require('decimal')
> +ffi = require('ffi')
> +
> +_ = box.schema.space.create('test', {engine=engine})
> +_ = box.space.test:create_index('pk', {parts={1,'decimal'}})
> +
> +test_run:cmd('setopt delimiter ";"')
> +for i = 0,16 do
> +    box.space.test:insert{decimal.new((i-8)/4)}
> +end;
> +test_run:cmd('setopt delimiter ""');
> +
> +box.space.test:select{}
> +
> +-- check invalid values
> +box.space.test:insert{1.23}
> +box.space.test:insert{'str'}
> +box.space.test:insert{ffi.new('uint64_t', 0)}
> +-- check duplicates
> +box.space.test:insert{decimal.new(0)}
> +
> +box.space.test.index.pk:drop()
> +
> +_ = box.space.test:create_index('pk', {parts={1, 'number'}})
> +
> +test_run:cmd('setopt delimiter ";"')
> +for i = 0, 32 do
> +    local val = (i - 16) / 8
> +    if i % 2 == 1 then val = decimal.new(val) end
> +    box.space.test:insert{val}
> +end;
> +test_run:cmd('setopt delimiter ""');
> +
> +box.space.test:select{}
> +
> +-- check duplicates
> +box.space.test:insert{-2}
> +box.space.test:insert{decimal.new(-2)}
> +box.space.test:insert{decimal.new(-1.875)}
> +box.space.test:insert{-1.875}
> +
> +box.space.test.index.pk:drop()
> +
> +_ = box.space.test:create_index('pk')
> +test_run:cmd('setopt delimiter ";"')
> +for i = 1,10 do
> +    box.space.test:insert{i, decimal.new(i/10)}
> +end;
> +test_run:cmd('setopt delimiter ""');
> +
> +-- a bigger test with a secondary index this time.
> +box.space.test:insert{11, 'str'}
> +box.space.test:insert{12, 0.63}
> +box.space.test:insert{13, 0.57}
> +box.space.test:insert{14, 0.33}
> +box.space.test:insert{16, 0.71}
> +
> +_ = box.space.test:create_index('sk', {parts={2, 'scalar'}})
> +box.space.test.index.sk:select{}
> +
> +box.space.test:drop()

Please add a test for space.format and index.alter.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [tarantool-patches] [PATCH 1/5] decimal: fix decimal.round() when scale == 0
  2019-07-17 15:33 ` [PATCH 1/5] decimal: fix decimal.round() when scale == 0 Serge Petrenko
  2019-07-24 12:10   ` Vladimir Davydov
@ 2019-07-24 23:02   ` Konstantin Osipov
  1 sibling, 0 replies; 16+ messages in thread
From: Konstantin Osipov @ 2019-07-24 23:02 UTC (permalink / raw)
  To: tarantool-patches; +Cc: vdavydov.dev, Serge Petrenko

* Serge Petrenko <sergepetrenko@tarantool.org> [19/07/17 18:34]:
> Fixes decimal.round() with zero scale, fixes an error with
> decimal.round() when rounding leads to a number with the same
> precision, for example, decimal.round(9.9, 0) -> 10.
> 
> Follow-up #692

lgtm

 

-- 
Konstantin Osipov, Moscow, Russia

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [tarantool-patches] [PATCH 2/5] decimal: allow to encode/decode decimals as MsgPack
  2019-07-17 15:33 ` [PATCH 2/5] decimal: allow to encode/decode decimals as MsgPack Serge Petrenko
  2019-07-24 16:19   ` Vladimir Davydov
@ 2019-07-24 23:08   ` Konstantin Osipov
  1 sibling, 0 replies; 16+ messages in thread
From: Konstantin Osipov @ 2019-07-24 23:08 UTC (permalink / raw)
  To: tarantool-patches; +Cc: vdavydov.dev, Serge Petrenko

* Serge Petrenko <sergepetrenko@tarantool.org> [19/07/17 18:34]:
> This patch adds the methods necessary to encode and decode decimals to
> MsgPack. MsgPack EXT type (MP_EXT) together with a new extension type
> MP_DECIMAL is used as a record header.
> 
> The decimal MsgPack representation looks like this:
> +--------+-------------------+------------+===============+
> | MP_EXT | length (optional) | MP_DECIMAL | PackedDecimal |
> +--------+-------------------+------------+===============+
> The whole record may be encoded and decoded with
> mp_encode_decimal() and mp_decode_decimal(). This is equivalent to
> performing mp_encode_extl()/mp_decode_extl() on the first 3 fields and
> decimal_pack/unpack() on the PackedDecimal field.

Please extend tarantool/msgpuck with libdecimal-agnostic data type
(e.g. struct mp_decimal { char data[] };, and encoders/decoders
from this struct.

Then use libdecimal api to fill this struct and pass to libmsgpuck
for encoding.

I think this patch is not worth having on its own, the next
logical step after patching msgpack is make sure net.box can
marshal decimals, and then - tarantool-python driver can read
them, then update test-run with the new python driver, then add a
new protocol test.


-- 
Konstantin Osipov, Moscow, Russia

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [tarantool-patches] [PATCH 3/5] lua: allow to encode and decode decimals as msgpack
  2019-07-17 15:33 ` [PATCH 3/5] lua: allow to encode and decode decimals as msgpack Serge Petrenko
  2019-07-24 14:11   ` Vladimir Davydov
@ 2019-07-24 23:10   ` Konstantin Osipov
  1 sibling, 0 replies; 16+ messages in thread
From: Konstantin Osipov @ 2019-07-24 23:10 UTC (permalink / raw)
  To: tarantool-patches; +Cc: vdavydov.dev, Serge Petrenko

* Serge Petrenko <sergepetrenko@tarantool.org> [19/07/17 18:34]:
> It is now possible to insert decimals into spaces, but only into
> unindexed fields.

OK, I get this. You need a quick encoder from Lua decimal into
space tuple, which bypasses an intermediate representation, for
this patch. Please merge the previous patch with this one then.

> 

-- 
Konstantin Osipov, Moscow, Russia

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [tarantool-patches] [PATCH 5/5] decimal: allow to index decimals
  2019-07-17 15:33 ` [PATCH 5/5] decimal: allow to index decimals Serge Petrenko
  2019-07-24 16:56   ` Vladimir Davydov
@ 2019-07-24 23:13   ` Konstantin Osipov
  2019-07-24 23:17   ` Konstantin Osipov
  2 siblings, 0 replies; 16+ messages in thread
From: Konstantin Osipov @ 2019-07-24 23:13 UTC (permalink / raw)
  To: tarantool-patches; +Cc: vdavydov.dev, Serge Petrenko

* Serge Petrenko <sergepetrenko@tarantool.org> [19/07/17 18:34]:
> Indices can now be built over decimal fields.
> A new field type - 'decimal' is introduced.
> Decimal values may be stored in 'decimal' columns, as well as in
> 'scalar' and 'any' columns.

At first I asked myself whether we should allow DECIMAL values in
NUMBER indexes. And then I recalled that Tarantool NUMBER is a
counterpart for Lua number, and lua 5.3 supports decimal values in
its number type.



-- 
Konstantin Osipov, Moscow, Russia

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [tarantool-patches] [PATCH 5/5] decimal: allow to index decimals
  2019-07-17 15:33 ` [PATCH 5/5] decimal: allow to index decimals Serge Petrenko
  2019-07-24 16:56   ` Vladimir Davydov
  2019-07-24 23:13   ` [tarantool-patches] " Konstantin Osipov
@ 2019-07-24 23:17   ` Konstantin Osipov
  2 siblings, 0 replies; 16+ messages in thread
From: Konstantin Osipov @ 2019-07-24 23:17 UTC (permalink / raw)
  To: tarantool-patches; +Cc: vdavydov.dev, Serge Petrenko

* Serge Petrenko <sergepetrenko@tarantool.org> [19/07/17 18:34]:
> + * Max and min decimal numbers whose integral parts fit
> + * in a hint value.
> + */
> +static const decimal_t HINT_VALUE_DECIMAL_MAX = {
> +	18,				/* decimal digits */
> +	0,				/* exponent */
> +	0,				/* no special bits. */
> +	{487, 423, 303, 752, 460, 576}	/* 576,460,752,303,423,488 = 2^59 - 1 */
> +					/* 59 == HINT_VALUE_BITS - 1 */
> +};

I don't get the comments here, could you please expand? 

> +static const decimal_t HINT_VALUE_DECIMAL_MIN = {
> +	18,				/* decimal digits */
> +	0,				/* exponent */
> +	0x80,				/* negative bit */
> +	{488, 423, 303, 752, 460, 576}	/* 576,460,752,303,423,488 = 2^59 */

Same here, some magic numbers, which don't match each other.
> +};

-- 
Konstantin Osipov, Moscow, Russia

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-07-24 23:17 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-17 15:33 [PATCH 0/5] Decimal indices Serge Petrenko
2019-07-17 15:33 ` [PATCH 1/5] decimal: fix decimal.round() when scale == 0 Serge Petrenko
2019-07-24 12:10   ` Vladimir Davydov
2019-07-24 23:02   ` [tarantool-patches] " Konstantin Osipov
2019-07-17 15:33 ` [PATCH 2/5] decimal: allow to encode/decode decimals as MsgPack Serge Petrenko
2019-07-24 16:19   ` Vladimir Davydov
2019-07-24 23:08   ` [tarantool-patches] " Konstantin Osipov
2019-07-17 15:33 ` [PATCH 3/5] lua: allow to encode and decode decimals as msgpack Serge Petrenko
2019-07-24 14:11   ` Vladimir Davydov
2019-07-24 23:10   ` [tarantool-patches] " Konstantin Osipov
2019-07-17 15:33 ` [PATCH 4/5] decimal: add conversions to (u)int64_t Serge Petrenko
2019-07-24 16:17   ` Vladimir Davydov
2019-07-17 15:33 ` [PATCH 5/5] decimal: allow to index decimals Serge Petrenko
2019-07-24 16:56   ` Vladimir Davydov
2019-07-24 23:13   ` [tarantool-patches] " Konstantin Osipov
2019-07-24 23:17   ` Konstantin Osipov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox