Tarantool development patches archive
 help / color / mirror / Atom feed
* [PATCH v2] box: add support for decimals in update ops
@ 2019-08-21  8:58 Serge Petrenko
  2019-08-21 10:45 ` Serge Petrenko
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Serge Petrenko @ 2019-08-21  8:58 UTC (permalink / raw)
  To: vdavydov.dev, kostja; +Cc: tarantool-patches, Serge Petrenko

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]
...
```
---
https://github.com/tarantool/tarantool/issues/4413
https://github.com/tarantool/tarantool/tree/sp/gh-4413-decimal-update

Changes in v2:
  - rebase on top of a new version of decimal
    indices patch, return to mp_type checks
    instead of mp_field_type


 src/box/errcode.h            |  2 +-
 src/box/tuple_update.c       | 94 ++++++++++++++++++++++++++++++++----
 test/box/misc.result         |  1 +
 test/box/tuple.result        | 58 ++++++++++++++++++++++
 test/box/tuple.test.lua      | 24 +++++++++
 test/engine/decimal.result   | 36 ++++++++++++++
 test/engine/decimal.test.lua | 11 +++++
 7 files changed, 216 insertions(+), 10 deletions(-)

diff --git a/src/box/errcode.h b/src/box/errcode.h
index 817275b97..46b0b365a 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 7a203ced8..18efcd735 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,18 @@ 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, &ret->dec);
+		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 +435,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 +475,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 +513,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 +536,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 +796,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 7a15dabf0..287d84e5b 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 a5010538d..895462518 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 8d4431bc6..9762fc8b3 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/engine/decimal.result b/test/engine/decimal.result
index 415868c89..2bf71bfec 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 3763bf0a3..3efd5c3e9 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()
-- 
2.20.1 (Apple Git-117)

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

* Re: [PATCH v2] box: add support for decimals in update ops
  2019-08-21  8:58 [PATCH v2] box: add support for decimals in update ops Serge Petrenko
@ 2019-08-21 10:45 ` Serge Petrenko
  2019-08-21 15:09 ` Vladimir Davydov
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Serge Petrenko @ 2019-08-21 10:45 UTC (permalink / raw)
  To: Vladimir Davydov, Konstantin Osipov; +Cc: tarantool-patches

Sorry, my bad. Minor fix below

diff --git a/src/box/tuple_update.c b/src/box/tuple_update.c
index 18efcd735..bbd95811e 100644
--- a/src/box/tuple_update.c
+++ b/src/box/tuple_update.c
@@ -301,7 +301,8 @@ mp_read_arith_arg(int index_base, struct update_op *op,
                switch (ext_type) {
                case MP_DECIMAL:
                        ret->type = AT_DECIMAL;
-                       decimal_unpack(expr, &ret->dec);
+                       decimal_unpack(expr, len, &ret->dec);
+                       break;
                default:
                        goto err;
                }

--
Serge Petrenko
sergepetrenko@tarantool.org




> 21 авг. 2019 г., в 11:58, Serge Petrenko <sergepetrenko@tarantool.org> написал(а):
> 
> 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]
> ...
> ```
> ---
> https://github.com/tarantool/tarantool/issues/4413
> https://github.com/tarantool/tarantool/tree/sp/gh-4413-decimal-update
> 
> Changes in v2:
>  - rebase on top of a new version of decimal
>    indices patch, return to mp_type checks
>    instead of mp_field_type
> 
> 
> src/box/errcode.h            |  2 +-
> src/box/tuple_update.c       | 94 ++++++++++++++++++++++++++++++++----
> test/box/misc.result         |  1 +
> test/box/tuple.result        | 58 ++++++++++++++++++++++
> test/box/tuple.test.lua      | 24 +++++++++
> test/engine/decimal.result   | 36 ++++++++++++++
> test/engine/decimal.test.lua | 11 +++++
> 7 files changed, 216 insertions(+), 10 deletions(-)
> 
> diff --git a/src/box/errcode.h b/src/box/errcode.h
> index 817275b97..46b0b365a 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 7a203ced8..18efcd735 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,18 @@ 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, &ret->dec);
> +		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 +435,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 +475,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 +513,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 +536,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 +796,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 7a15dabf0..287d84e5b 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 a5010538d..895462518 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 8d4431bc6..9762fc8b3 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/engine/decimal.result b/test/engine/decimal.result
> index 415868c89..2bf71bfec 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 3763bf0a3..3efd5c3e9 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()
> -- 
> 2.20.1 (Apple Git-117)
> 

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

* Re: [PATCH v2] box: add support for decimals in update ops
  2019-08-21  8:58 [PATCH v2] box: add support for decimals in update ops Serge Petrenko
  2019-08-21 10:45 ` Serge Petrenko
@ 2019-08-21 15:09 ` Vladimir Davydov
  2019-08-21 21:37 ` Konstantin Osipov
  2019-08-22 10:34 ` [tarantool-patches] " Kirill Yukhin
  3 siblings, 0 replies; 7+ messages in thread
From: Vladimir Davydov @ 2019-08-21 15:09 UTC (permalink / raw)
  To: Serge Petrenko; +Cc: kostja, tarantool-patches

Ack

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

* Re: [PATCH v2] box: add support for decimals in update ops
  2019-08-21  8:58 [PATCH v2] box: add support for decimals in update ops Serge Petrenko
  2019-08-21 10:45 ` Serge Petrenko
  2019-08-21 15:09 ` Vladimir Davydov
@ 2019-08-21 21:37 ` Konstantin Osipov
  2019-08-22  8:54   ` Serge Petrenko
  2019-08-22 10:34 ` [tarantool-patches] " Kirill Yukhin
  3 siblings, 1 reply; 7+ messages in thread
From: Konstantin Osipov @ 2019-08-21 21:37 UTC (permalink / raw)
  To: Serge Petrenko; +Cc: vdavydov.dev, tarantool-patches

* Serge Petrenko <sergepetrenko@tarantool.org> [19/08/21 12:03]:
> 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:

Please document how type arithmetics work, and cover it with
tests.

-- 
Konstantin Osipov, Moscow, Russia

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

* Re: [PATCH v2] box: add support for decimals in update ops
  2019-08-21 21:37 ` Konstantin Osipov
@ 2019-08-22  8:54   ` Serge Petrenko
  2019-08-22 10:07     ` Konstantin Osipov
  0 siblings, 1 reply; 7+ messages in thread
From: Serge Petrenko @ 2019-08-22  8:54 UTC (permalink / raw)
  To: Konstantin Osipov; +Cc: Vladimir Davydov, tarantool-patches





> 22 авг. 2019 г., в 0:37, Konstantin Osipov <kostja@tarantool.org> написал(а):
> 
> * Serge Petrenko <sergepetrenko@tarantool.org> [19/08/21 12:03]:
>> 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:
> 
> Please document how type arithmetics work, and cover it with
> tests.

Hi! I added the following text to the docbot request in commit message:

«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.»



Here’s the tests diff:
diff --git a/test/box/update.result b/test/box/update.result
index a3f731b55..cc8fa7ad0 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 c455bc1e1..ac7698ce9 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



> 
> -- 
> Konstantin Osipov, Moscow, Russia

--
Serge Petrenko
sergepetrenko@tarantool.org

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

* Re: [PATCH v2] box: add support for decimals in update ops
  2019-08-22  8:54   ` Serge Petrenko
@ 2019-08-22 10:07     ` Konstantin Osipov
  0 siblings, 0 replies; 7+ messages in thread
From: Konstantin Osipov @ 2019-08-22 10:07 UTC (permalink / raw)
  To: Serge Petrenko; +Cc: Vladimir Davydov, tarantool-patches

* Serge Petrenko <sergepetrenko@tarantool.org> [19/08/22 11:59]:
> 
> >> 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:
> > 
> > Please document how type arithmetics work, and cover it with
> > tests.
> 
> Hi! I added the following text to the docbot request in commit message:
> 
> «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.»

thanks. lgtm then.

-- 
Konstantin Osipov, Moscow, Russia

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

* Re: [tarantool-patches] [PATCH v2] box: add support for decimals in update ops
  2019-08-21  8:58 [PATCH v2] box: add support for decimals in update ops Serge Petrenko
                   ` (2 preceding siblings ...)
  2019-08-21 21:37 ` Konstantin Osipov
@ 2019-08-22 10:34 ` Kirill Yukhin
  3 siblings, 0 replies; 7+ messages in thread
From: Kirill Yukhin @ 2019-08-22 10:34 UTC (permalink / raw)
  To: tarantool-patches; +Cc: vdavydov.dev, kostja, Serge Petrenko

Hello,

On 21 Aug 11:58, Serge Petrenko wrote:
> Closes #4413
> 
> @TarantoolBot document
> Title: update operations on decimal fields.

I've checked the patch into master.

--
Regards, Kirill Yukhin

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

end of thread, other threads:[~2019-08-22 10:34 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-21  8:58 [PATCH v2] box: add support for decimals in update ops Serge Petrenko
2019-08-21 10:45 ` Serge Petrenko
2019-08-21 15:09 ` Vladimir Davydov
2019-08-21 21:37 ` Konstantin Osipov
2019-08-22  8:54   ` Serge Petrenko
2019-08-22 10:07     ` Konstantin Osipov
2019-08-22 10:34 ` [tarantool-patches] " Kirill Yukhin

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