[PATCH v2] box: add support for decimals in update ops

Serge Petrenko sergepetrenko at tarantool.org
Wed Aug 21 13:45:23 MSK 2019


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 at tarantool.org




> 21 авг. 2019 г., в 11:58, Serge Petrenko <sergepetrenko at 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)
> 




More information about the Tarantool-patches mailing list