From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.11\)) Subject: Re: [PATCH v2] box: add support for decimals in update ops From: Serge Petrenko In-Reply-To: <20190821085808.12236-1-sergepetrenko@tarantool.org> Date: Wed, 21 Aug 2019 13:45:23 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <20190821085808.12236-1-sergepetrenko@tarantool.org> To: Vladimir Davydov , Konstantin Osipov Cc: tarantool-patches@freelists.org List-ID: 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 =3D AT_DECIMAL; - decimal_unpack(expr, &ret->dec); + decimal_unpack(expr, len, &ret->dec); + break; default: goto err; } -- Serge Petrenko sergepetrenko@tarantool.org > 21 =D0=B0=D0=B2=D0=B3. 2019 =D0=B3., =D0=B2 11:58, Serge Petrenko = =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0= =B0): >=20 > Closes #4413 >=20 > @TarantoolBot document > Title: update operations on decimal fields. >=20 > tuple:update and space:update now support deicmal operands for > arithmetic operations ('+' and '-'). The syntax is as usual: > ``` > d =3D box.tuple.new(decimal.new('1')) > --- > ... > d:update{{'+', 1, decimal.new('0.5')}} > --- > - [1.5] > ... > ``` >=20 > Insertion ('!') and assignment ('=3D') are also supported: > ``` > a =3D decimal.new('1') > --- > ... > b =3D decimal.new('1e10') > --- > ... > c =3D decimal.new('1e-10') > --- > ... > d =3D box.tuple.new{5, a, 6, b, 7, c, "string"} > --- > ... > d > --- > - [5, 1, 6, 10000000000, 7, 0.0000000001, 'string'] > ... >=20 > d:update{{'!', 3, dec.new('1234.5678')}} > --- > - [5, 1, 1234.5678, 6, 10000000000, 7, 0.0000000001, 'string'] > ... > d:update{{'=3D', -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 >=20 > Changes in v2: > - rebase on top of a new version of decimal > indices patch, return to mp_type checks > instead of mp_field_type >=20 >=20 > 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(-) >=20 > 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" >=20 >=20 > /** 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 =3D 0, /* MP_DOUBLE */ > - AT_FLOAT =3D 1, /* MP_FLOAT */ > - AT_INT =3D 2 /* MP_INT/MP_UINT */ > + AT_DECIMAL =3D 0, /* MP_EXT + MP_DECIMAL */ > + AT_DOUBLE =3D 1, /* MP_DOUBLE */ > + AT_FLOAT =3D 2, /* MP_FLOAT */ > + AT_INT =3D 3 /* MP_INT/MP_UINT */ > }; >=20 > /** > @@ -157,6 +160,7 @@ struct op_arith_arg { > double dbl; > float flt; > struct int96_num int96; > + decimal_t dec; > }; > }; >=20 > @@ -291,8 +295,18 @@ mp_read_arith_arg(int index_base, struct = update_op *op, > } else if (mp_typeof(**expr) =3D=3D MP_FLOAT) { > ret->type =3D AT_FLOAT; > ret->flt =3D mp_decode_float(expr); > + } else if (mp_typeof(**expr) =3D=3D MP_EXT) { > + int8_t ext_type; > + uint32_t len =3D mp_decode_extl(expr, &ext_type); > + switch (ext_type) { > + case MP_DECIMAL: > + ret->type =3D 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) > } > } >=20 > +static inline decimal_t * > +cast_arith_arg_to_decimal(struct op_arith_arg arg, decimal_t *dec) > +{ > + decimal_t *ret; > + if (arg.type =3D=3D AT_DECIMAL) { > + *dec =3D arg.dec; > + return dec; > + } else if (arg.type =3D=3D AT_DOUBLE) { > + ret =3D decimal_from_double(dec, arg.dbl); > + } else if (arg.type =3D=3D AT_FLOAT) { > + ret =3D decimal_from_double(dec, arg.flt); > + } else { > + assert(arg.type =3D=3D AT_INT); > + if (int96_is_uint64(&arg.int96)) { > + uint64_t val =3D = int96_extract_uint64(&arg.int96); > + ret =3D decimal_from_uint64(dec, val); > + } else { > + assert(int96_is_neg_int64(&arg.int96)); > + int64_t val =3D = int96_extract_neg_int64(&arg.int96); > + ret =3D 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 =3D=3D AT_DOUBLE) { > return mp_sizeof_double(arg.dbl); > - } else { > - assert(arg.type =3D=3D AT_FLOAT); > + } else if (arg.type =3D=3D AT_FLOAT) { > return mp_sizeof_float(arg.flt); > + } else { > + assert(arg.type =3D=3D AT_DECIMAL); > + return mp_sizeof_decimal(&arg.dec); > } > } >=20 > @@ -471,7 +513,7 @@ make_arith_operation(struct op_arith_arg arg1, = struct op_arith_arg arg2, > } > *ret =3D arg1; > return 0; > - } else { > + } else if (lowest_type >=3D AT_DOUBLE) { > /* At least one of operands is double or float */ > double a =3D cast_arith_arg_to_double(arg1); > double b =3D 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 =3D AT_FLOAT; > ret->flt =3D (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) =3D=3D NULL) { > + diag_set(ClientError, = ER_UPDATE_DECIMAL_OVERFLOW, > + opcode, err_fieldno); > + return -1; > + } > + break; > + case '-': > + if (decimal_sub(&c, &a, &b) =3D=3D 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 =3D AT_DECIMAL; > + ret->dec =3D c; > } > return 0; > } > @@ -722,9 +796,11 @@ store_op_arith(struct op_arith_arg *arg, const = char *in, char *out) > } > } else if (arg->type =3D=3D AT_DOUBLE) { > mp_encode_double(out, arg->dbl); > - } else { > - assert(arg->type =3D=3D AT_FLOAT); > + } else if (arg->type =3D=3D AT_FLOAT) { > mp_encode_float(out, arg->flt); > + } else { > + assert (arg->type =3D=3D AT_DECIMAL); > + mp_encode_decimal(out, &arg->dec); > } > } >=20 > 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{{'=3D', -1, = dec.new('0.12345678910111213')}} > --- > - [5, 1, 6, 10000000000, 7, 0.0000000001, 0.12345678910111213] > ... > +-- > +-- gh-4413: tuple:update arithmetic for decimals > +-- > +ffi =3D require('ffi') > +--- > +... > +d =3D 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 =3D 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{{'=3D', -1, dec.new('0.12345678910111213')}} > + > +-- > +-- gh-4413: tuple:update arithmetic for decimals > +-- > +ffi =3D require('ffi') > + > +d =3D 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 =3D 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] > | ... >=20 > +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=3D{2, = 'number'}} > box.space.test:insert{2, -5} > box.space.test.index.sk:select{} >=20 > +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() > --=20 > 2.20.1 (Apple Git-117) >=20