From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Serge Petrenko Subject: [PATCH v2] box: add support for decimals in update ops Date: Wed, 21 Aug 2019 11:58:08 +0300 Message-Id: <20190821085808.12236-1-sergepetrenko@tarantool.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit To: vdavydov.dev@gmail.com, kostja@tarantool.org Cc: tarantool-patches@freelists.org, Serge Petrenko List-ID: 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 #include #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)