From 8317189a9b1f8351dabaecc8216a1c65b175d227 Mon Sep 17 00:00:00 2001
From: Serge Petrenko <sergepetrenko@tarantool.org>
Date: Fri, 9 Aug 2019 15:39:33 +0300
Subject: [PATCH] box: add support for decimals in update ops

Closes #4413

@TarantoolBot document
Title: update operations on decimal fields.

tuple:update and space:update now support deicmal operands for
arithmetic operations ('+' and '-'). The syntax is as usual:
```
d = box.tuple.new(decimal.new('1'))
---
...
d:update{{'+', 1, decimal.new('0.5')}}
---
- [1.5]
...
```

Insertion ('!') and assignment ('=') are also supported:
```
a = decimal.new('1')
---
...
b = decimal.new('1e10')
---
...
c = decimal.new('1e-10')
---
...
d = box.tuple.new{5, a, 6, b, 7, c, "string"}
---
...
d
---
- [5, 1, 6, 10000000000, 7, 0.0000000001, 'string']
...

d:update{{'!', 3, dec.new('1234.5678')}}
---
- [5, 1, 1234.5678, 6, 10000000000, 7, 0.0000000001, 'string']
...
d:update{{'=', -1, dec.new('0.12345678910111213')}}
---
- [5, 1, 6, 10000000000, 7, 0.0000000001, 0.12345678910111213]

When performing an arithmetic operation ('+', '-'), where either the
updated field or the operand is decimal, the result will be decimal.

When both the updated field and the operand are decimal, the result
will, of course, be decimal.

...
```
---
 src/box/errcode.h            |  2 +-
 src/box/tuple_update.c       | 95 ++++++++++++++++++++++++++++++++----
 test/box/misc.result         |  1 +
 test/box/tuple.result        | 58 ++++++++++++++++++++++
 test/box/tuple.test.lua      | 24 +++++++++
 test/box/update.result       | 40 +++++++++++++++
 test/box/update.test.lua     | 11 +++++
 test/engine/decimal.result   | 36 ++++++++++++++
 test/engine/decimal.test.lua | 11 +++++
 9 files changed, 268 insertions(+), 10 deletions(-)

diff --git a/src/box/errcode.h b/src/box/errcode.h
index 817275b972..46b0b365ad 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -212,7 +212,7 @@ struct errcode_record {
 	/*157 */_(ER_SQL_BIND_TYPE,             "Bind value type %s for parameter %s is not supported") \
 	/*158 */_(ER_SQL_BIND_PARAMETER_MAX,    "SQL bind parameter limit reached: %d") \
 	/*159 */_(ER_SQL_EXECUTE,               "Failed to execute SQL statement: %s") \
-	/*160 */_(ER_UNUSED,			"") \
+	/*160 */_(ER_UPDATE_DECIMAL_OVERFLOW,	"Decimal overflow when performing operation '%c' on field %u") \
 	/*161 */_(ER_SQL_BIND_NOT_FOUND,	"Parameter %s was not found in the statement") \
 	/*162 */_(ER_ACTION_MISMATCH,		"Field %s contains %s on conflict action, but %s in index parts") \
 	/*163 */_(ER_VIEW_MISSING_SQL,		"Space declared as a view must have SQL statement") \
diff --git a/src/box/tuple_update.c b/src/box/tuple_update.c
index 7a203ced82..bbd95811ec 100644
--- a/src/box/tuple_update.c
+++ b/src/box/tuple_update.c
@@ -43,6 +43,8 @@
 #include <bit/int96.h>
 #include <salad/rope.h>
 #include "column_mask.h"
+#include "mp_extension_types.h"
+#include "mp_decimal.h"
 
 
 /** UPDATE request implementation.
@@ -125,9 +127,10 @@ struct op_del_arg {
  * MsgPack codes are not used to simplify type calculation.
  */
 enum arith_type {
-	AT_DOUBLE = 0, /* MP_DOUBLE */
-	AT_FLOAT = 1, /* MP_FLOAT */
-	AT_INT = 2 /* MP_INT/MP_UINT */
+	AT_DECIMAL = 0, /* MP_EXT + MP_DECIMAL */
+	AT_DOUBLE = 1, /* MP_DOUBLE */
+	AT_FLOAT = 2, /* MP_FLOAT */
+	AT_INT = 3 /* MP_INT/MP_UINT */
 };
 
 /**
@@ -157,6 +160,7 @@ struct op_arith_arg {
 		double dbl;
 		float flt;
 		struct int96_num int96;
+		decimal_t dec;
 	};
 };
 
@@ -291,8 +295,19 @@ mp_read_arith_arg(int index_base, struct update_op *op,
 	} else if (mp_typeof(**expr) == MP_FLOAT) {
 		ret->type = AT_FLOAT;
 		ret->flt = mp_decode_float(expr);
+	} else if (mp_typeof(**expr) == MP_EXT) {
+		int8_t ext_type;
+		uint32_t len = mp_decode_extl(expr, &ext_type);
+		switch (ext_type) {
+		case MP_DECIMAL:
+			ret->type = AT_DECIMAL;
+			decimal_unpack(expr, len, &ret->dec);
+			break;
+		default:
+			goto err;
+		}
 	} else {
-		diag_set(ClientError, ER_UPDATE_ARG_TYPE, (char)op->opcode,
+err:		diag_set(ClientError, ER_UPDATE_ARG_TYPE, (char)op->opcode,
 			 index_base + op->field_no, "a number");
 		return -1;
 	}
@@ -421,6 +436,32 @@ cast_arith_arg_to_double(struct op_arith_arg arg)
 	}
 }
 
+static inline decimal_t *
+cast_arith_arg_to_decimal(struct op_arith_arg arg, decimal_t *dec)
+{
+	decimal_t *ret;
+	if (arg.type == AT_DECIMAL) {
+		*dec = arg.dec;
+		return dec;
+	} else if (arg.type == AT_DOUBLE) {
+		ret = decimal_from_double(dec, arg.dbl);
+	} else if (arg.type == AT_FLOAT) {
+		ret = decimal_from_double(dec, arg.flt);
+	} else {
+		assert(arg.type == AT_INT);
+		if (int96_is_uint64(&arg.int96)) {
+			uint64_t val = int96_extract_uint64(&arg.int96);
+			ret = decimal_from_uint64(dec, val);
+		} else {
+			assert(int96_is_neg_int64(&arg.int96));
+			int64_t val = int96_extract_neg_int64(&arg.int96);
+			ret = decimal_from_int64(dec, val);
+		}
+	}
+
+	return ret;
+}
+
 /** Return the MsgPack size of an arithmetic operation result. */
 static inline uint32_t
 mp_sizeof_op_arith_arg(struct op_arith_arg arg)
@@ -435,9 +476,11 @@ mp_sizeof_op_arith_arg(struct op_arith_arg arg)
 		}
 	} else if (arg.type == AT_DOUBLE) {
 		return mp_sizeof_double(arg.dbl);
-	} else {
-		assert(arg.type == AT_FLOAT);
+	} else if (arg.type == AT_FLOAT) {
 		return mp_sizeof_float(arg.flt);
+	} else {
+		assert(arg.type == AT_DECIMAL);
+		return mp_sizeof_decimal(&arg.dec);
 	}
 }
 
@@ -471,7 +514,7 @@ make_arith_operation(struct op_arith_arg arg1, struct op_arith_arg arg2,
 		}
 		*ret = arg1;
 		return 0;
-	} else {
+	} else if (lowest_type >= AT_DOUBLE) {
 		/* At least one of operands is double or float */
 		double a = cast_arith_arg_to_double(arg1);
 		double b = cast_arith_arg_to_double(arg2);
@@ -494,6 +537,38 @@ make_arith_operation(struct op_arith_arg arg1, struct op_arith_arg arg2,
 			ret->type = AT_FLOAT;
 			ret->flt = (float)c;
 		}
+	} else {
+		/* At least one of the operands is decimal. */
+		decimal_t a, b, c;
+		if (! cast_arith_arg_to_decimal(arg1, &a) ||
+		    ! cast_arith_arg_to_decimal(arg2, &b)) {
+			diag_set(ClientError, ER_UPDATE_ARG_TYPE, (char)opcode,
+				 err_fieldno, "a number convertible to decimal.");
+			return -1;
+		}
+
+		switch(opcode) {
+		case '+':
+			if (decimal_add(&c, &a, &b) == NULL) {
+				diag_set(ClientError, ER_UPDATE_DECIMAL_OVERFLOW,
+					 opcode, err_fieldno);
+				return -1;
+			}
+			break;
+		case '-':
+			if (decimal_sub(&c, &a, &b) == NULL) {
+				diag_set(ClientError, ER_UPDATE_DECIMAL_OVERFLOW,
+					 opcode, err_fieldno);
+				return -1;
+			}
+			break;
+		default:
+			diag_set(ClientError, ER_UPDATE_ARG_TYPE, (char)opcode,
+				 err_fieldno);
+			return -1;
+		}
+		ret->type = AT_DECIMAL;
+		ret->dec = c;
 	}
 	return 0;
 }
@@ -722,9 +797,11 @@ store_op_arith(struct op_arith_arg *arg, const char *in, char *out)
 		}
 	} else if (arg->type == AT_DOUBLE) {
 		mp_encode_double(out, arg->dbl);
-	} else {
-		assert(arg->type == AT_FLOAT);
+	} else if (arg->type == AT_FLOAT) {
 		mp_encode_float(out, arg->flt);
+	} else {
+		assert (arg->type == AT_DECIMAL);
+		mp_encode_decimal(out, &arg->dec);
 	}
 }
 
diff --git a/test/box/misc.result b/test/box/misc.result
index 7a15dabf07..287d84e5bb 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -490,6 +490,7 @@ t;
   157: box.error.SQL_BIND_TYPE
   158: box.error.SQL_BIND_PARAMETER_MAX
   159: box.error.SQL_EXECUTE
+  160: box.error.UPDATE_DECIMAL_OVERFLOW
   161: box.error.SQL_BIND_NOT_FOUND
   162: box.error.ACTION_MISMATCH
   163: box.error.VIEW_MISSING_SQL
diff --git a/test/box/tuple.result b/test/box/tuple.result
index a5010538db..895462518d 100644
--- a/test/box/tuple.result
+++ b/test/box/tuple.result
@@ -1337,3 +1337,61 @@ d:update{{'=', -1, dec.new('0.12345678910111213')}}
 ---
 - [5, 1, 6, 10000000000, 7, 0.0000000001, 0.12345678910111213]
 ...
+--
+-- gh-4413: tuple:update arithmetic for decimals
+--
+ffi = require('ffi')
+---
+...
+d = box.tuple.new(dec.new('1'))
+---
+...
+d:update{{'+', 1, dec.new('0.5')}}
+---
+- [1.5]
+...
+d:update{{'-', 1, dec.new('0.5')}}
+---
+- [0.5]
+...
+d:update{{'+', 1, 1.36}}
+---
+- [2.36]
+...
+d:update{{'+', 1, ffi.new('uint64_t', 1712)}}
+---
+- [1713]
+...
+d:update{{'-', 1, ffi.new('float', 635)}}
+---
+- [-634]
+...
+-- test erroneous values
+-- nan
+d:update{{'+', 1, 0/0}}
+---
+- error: 'Argument type in operation ''+'' on field 1 does not match field type: expected
+    a number convertible to decimal.'
+...
+-- inf
+d:update{{'-', 1, 1/0}}
+---
+- error: 'Argument type in operation ''-'' on field 1 does not match field type: expected
+    a number convertible to decimal.'
+...
+-- decimal overflow
+d = box.tuple.new(dec.new('9e37'))
+---
+...
+d
+---
+- [90000000000000000000000000000000000000]
+...
+d:update{{'+', 1, dec.new('1e37')}}
+---
+- error: Decimal overflow when performing operation '+' on field 1
+...
+d:update{{'-', 1, dec.new('1e37')}}
+---
+- [80000000000000000000000000000000000000]
+...
diff --git a/test/box/tuple.test.lua b/test/box/tuple.test.lua
index 8d4431bc66..9762fc8b3e 100644
--- a/test/box/tuple.test.lua
+++ b/test/box/tuple.test.lua
@@ -449,3 +449,27 @@ msgpack.decode(msgpackffi.encode(d))
 d:bsize()
 d:update{{'!', 3, dec.new('1234.5678')}}
 d:update{{'=', -1, dec.new('0.12345678910111213')}}
+
+--
+-- gh-4413: tuple:update arithmetic for decimals
+--
+ffi = require('ffi')
+
+d = box.tuple.new(dec.new('1'))
+d:update{{'+', 1, dec.new('0.5')}}
+d:update{{'-', 1, dec.new('0.5')}}
+d:update{{'+', 1, 1.36}}
+d:update{{'+', 1, ffi.new('uint64_t', 1712)}}
+d:update{{'-', 1, ffi.new('float', 635)}}
+
+-- test erroneous values
+-- nan
+d:update{{'+', 1, 0/0}}
+-- inf
+d:update{{'-', 1, 1/0}}
+
+-- decimal overflow
+d = box.tuple.new(dec.new('9e37'))
+d
+d:update{{'+', 1, dec.new('1e37')}}
+d:update{{'-', 1, dec.new('1e37')}}
diff --git a/test/box/update.result b/test/box/update.result
index a3f731b55d..cc8fa7ad0c 100644
--- a/test/box/update.result
+++ b/test/box/update.result
@@ -677,6 +677,46 @@ s:update({0}, {{'+', 2, ffi.new("float", 1.2)}}) -- float + float = float 1.2
 ---
 - [0, 1.2000000476837]
 ...
+-- decimal
+decimal = require('decimal')
+---
+...
+s:replace{0, decimal.new("2.000")}
+---
+- [0, 2.000]
+...
+s:update({0}, {{'+', 2, 2ULL}}) -- decimal + unsigned = decimal 4.000
+---
+- [0, 4.000]
+...
+s:update({0}, {{'+', 2, -4LL}}) -- decimal + signed = decimal 0.000
+---
+- [0, 0.000]
+...
+s:update({0}, {{'+', 2, 2}})  -- decimal + number = decimal 2.000
+---
+- [0, 2.000]
+...
+s:update({0}, {{'-', 2, 2}}) -- decimal - number = decimal 0.000
+---
+- [0, 0.000]
+...
+s:update({0}, {{'-', 2, ffi.new('float', 2)}}) -- decimal - float = decimal -2.000
+---
+- [0, -2.000]
+...
+s:update({0}, {{'-', 2, ffi.new('double', 2)}}) -- decimal - double = decimal -4.000
+---
+- [0, -4.000]
+...
+s:update({0}, {{'+', 2, decimal.new(4)}}) -- decimal + decimal = decimal 0.000
+---
+- [0, 0.000]
+...
+s:update({0}, {{'-', 2, decimal.new(2)}}) -- decimal - decimal = decimal -2.000
+---
+- [0, -2.000]
+...
 -- overflow --
 s:replace{0, 0xfffffffffffffffeull}
 ---
diff --git a/test/box/update.test.lua b/test/box/update.test.lua
index c455bc1e1e..ac7698ce9f 100644
--- a/test/box/update.test.lua
+++ b/test/box/update.test.lua
@@ -210,6 +210,17 @@ s:update({0}, {{'-', 2, ffi.new("float", 1.5)}}) -- float - float = float 5.5
 s:update({0}, {{'+', 2, ffi.new("float", 3.5)}}) -- float + float = float 9
 s:update({0}, {{'-', 2, ffi.new("float", 9)}}) -- float + float = float 0
 s:update({0}, {{'+', 2, ffi.new("float", 1.2)}}) -- float + float = float 1.2
+-- decimal
+decimal = require('decimal')
+s:replace{0, decimal.new("2.000")}
+s:update({0}, {{'+', 2, 2ULL}}) -- decimal + unsigned = decimal 4.000
+s:update({0}, {{'+', 2, -4LL}}) -- decimal + signed = decimal 0.000
+s:update({0}, {{'+', 2, 2}})  -- decimal + number = decimal 2.000
+s:update({0}, {{'-', 2, 2}}) -- decimal - number = decimal 0.000
+s:update({0}, {{'-', 2, ffi.new('float', 2)}}) -- decimal - float = decimal -2.000
+s:update({0}, {{'-', 2, ffi.new('double', 2)}}) -- decimal - double = decimal -4.000
+s:update({0}, {{'+', 2, decimal.new(4)}}) -- decimal + decimal = decimal 0.000
+s:update({0}, {{'-', 2, decimal.new(2)}}) -- decimal - decimal = decimal -2.000
 -- overflow --
 s:replace{0, 0xfffffffffffffffeull}
 s:update({0}, {{'+', 2, 1}}) -- ok
diff --git a/test/engine/decimal.result b/test/engine/decimal.result
index 415868c896..2bf71bfecb 100644
--- a/test/engine/decimal.result
+++ b/test/engine/decimal.result
@@ -351,6 +351,42 @@ box.space.test.index.sk:select{}
  |   - [3, 3]
  | ...
 
+box.space.test:truncate()
+ | ---
+ | ...
+
+-- test update operations
+box.space.test:insert{1, decimal.new(1.10)}
+ | ---
+ | - [1, 1.1]
+ | ...
+box.space.test:insert{2, 2}
+ | ---
+ | - [2, 2]
+ | ...
+box.space.test:update(1, {{'+', 2, 3.1}})
+ | ---
+ | - [1, 4.2]
+ | ...
+box.space.test.index.sk:select{}
+ | ---
+ | - - [2, 2]
+ |   - [1, 4.2]
+ | ...
+box.space.test:update(1, {{'-', 2, decimal.new(3.3)}})
+ | ---
+ | - [1, 0.9]
+ | ...
+box.space.test:update(2, {{'+', 2, decimal.new(0.1)}})
+ | ---
+ | - [2, 2.1]
+ | ...
+box.space.test.index.sk:select{}
+ | ---
+ | - - [1, 0.9]
+ |   - [2, 2.1]
+ | ...
+
 box.space.test:drop()
  | ---
  | ...
diff --git a/test/engine/decimal.test.lua b/test/engine/decimal.test.lua
index 3763bf0a35..3efd5c3e98 100644
--- a/test/engine/decimal.test.lua
+++ b/test/engine/decimal.test.lua
@@ -101,4 +101,15 @@ box.space.test.index.sk:alter{parts={2, 'number'}}
 box.space.test:insert{2, -5}
 box.space.test.index.sk:select{}
 
+box.space.test:truncate()
+
+-- test update operations
+box.space.test:insert{1, decimal.new(1.10)}
+box.space.test:insert{2, 2}
+box.space.test:update(1, {{'+', 2, 3.1}})
+box.space.test.index.sk:select{}
+box.space.test:update(1, {{'-', 2, decimal.new(3.3)}})
+box.space.test:update(2, {{'+', 2, decimal.new(0.1)}})
+box.space.test.index.sk:select{}
+
 box.space.test:drop()
-- 
GitLab