<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">Hi! Thank you for review!<div class="">I addressed your comments, and pushed the new version on the branch.</div><div class="">The diff is below. It is rather big, but I feel like it belongs here, rather than in</div><div class="">a `v2` letter, since the changes are very minor.<br class=""><div class="">
<div dir="auto" style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div>--</div><div>Serge Petrenko</div><div><a href="mailto:sergepetrenko@tarantool.org" class="">sergepetrenko@tarantool.org</a></div><div class=""><br class=""></div></div><br class="Apple-interchange-newline"><br class="Apple-interchange-newline">
</div>
<br class=""><blockquote type="cite" class="">21 июня 2019 г., в 18:53, Vladimir Davydov <<a href="mailto:vdavydov.dev@gmail.com" class="">vdavydov.dev@gmail.com</a>> написал(а):<br class=""><br class="">On Wed, Jun 19, 2019 at 06:58:05PM +0300, Serge Petrenko wrote:<br class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">Add a decimal library to lua.<br class=""><br class="">Part of #692<br class=""><br class="">@TarantoolBot document<br class="">Title: Document decimal module in lua.<br class=""><br class="">First of all, you have to require the package via<br class="">`decimal = require('decimal')`<br class="">Now you can construct decimals via `tonumber` method.<br class="">Decimals may be constructed from lua numbers, strings, unsigned and<br class="">signed 64 bit integers.<br class="">Decimal is a fixed-point type with maximum 38 digits of precision. All<br class="">the calculations are exact, so, be careful when constructing decimals<br class="">from lua numbers: they may hold only 15 decimal digits of precision.<br class="">You are advised to construct decimals from strings, since strings<br class="">represent decimals exactly, and vice versa.<br class=""><br class="">```<br class="">a = decimal.tonumber(123e-7)<br class="">b = decimal.tonumber('123.456')<br class="">c = decimal.tonumber('123.456e2')<br class="">d = decimal.tonumber(123ULL)<br class="">e = decimal.tonumber(2)<br class="">```<br class=""></blockquote><br class="">tonumber is a confusing name IMO. Let's rename it to decimal.new()<br class=""></blockquote><div class=""><br class=""></div>Ok</div><div class=""><br class=""><blockquote type="cite" class=""><br class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">The allowed operations are addition, subtraction, division,<br class="">multiplication and power. If at least one of the operands is decimal,<br class="">decimal operations are performed. The other operand may be either<br class="">decimal or string, containing a number representation, or a lua number.<br class="">When the operation is called as `decimal.opname`, both operands may be<br class="">strings or lua numbers, e.g. `decimal.add(23, '123.456') == 146.456`:<br class="">```<br class=""></blockquote><br class="">Can these functions fail (overflow, underflow)? What happens on error?<br class="">Please mention in this document.<br class=""></blockquote><div class=""><br class=""></div>I made the message more informative.</div><div class=""><br class=""><blockquote type="cite" class=""><br class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">tarantool> a + b<br class="">---<br class="">- '123.456012300000000'<br class="">...<br class=""><br class="">tarantool> decimal.add(a,b)<br class="">---<br class="">- '123.456012300000000'<br class="">...<br class=""><br class="">tarantool> c - d<br class="">---<br class="">- '12222.6'<br class="">...<br class=""><br class="">tarantool> decimal.sub(c,d)<br class=""></blockquote><br class="">I don't think we need decimal.add/sub/div/mul methods as long as we have<br class="">corresponding operators.<br class=""></blockquote><div class=""><br class=""></div>No problem.</div><div class=""><br class=""><blockquote type="cite" class=""><br class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">The following math functions are also supported:<br class="">log10, ln, exp, sqrt, pow. When specified as<br class="">`decimal.opname()`, operations may be performed on<br class="">strings and lua numbers.<br class="">```<br class="">f = decimal.tonumber(100)<br class="">tarantool> f:log10()<br class=""></blockquote><br class="">This looks weird. I think we should only allow decimal.log10(x),<br class="">not x:log10().<br class=""></blockquote><div class=""><br class=""></div>All the math functions are now accessed as decimal.func(), decimal.scale() and</div><div class="">decimal.precision() are added.</div><div class=""><br class=""><blockquote type="cite" class=""><br class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">---<br class="">src/CMakeLists.txt | 1 +<br class="">src/lua/decimal.c | 327 ++++++++++++++++++++++++++++++++++++++<br class="">src/lua/decimal.h | 39 +++++<br class="">src/lua/init.c | 2 +<br class="">test/app/decimal.result | 172 ++++++++++++++++++++<br class="">test/app/decimal.test.lua | 50 ++++++<br class="">6 files changed, 591 insertions(+)<br class="">create mode 100644 src/lua/decimal.c<br class="">create mode 100644 src/lua/decimal.h<br class="">create mode 100644 test/app/decimal.result<br class="">create mode 100644 test/app/decimal.test.lua<br class=""></blockquote><br class="">Please consider implementing this using Lua ffi. Take a look at<br class="">src/lua/uuid.lua for example. They say that ffi implementation is<br class="">generally faster than Lua C, because it doesn't break JIT traces.<br class="">It might be worth benchmarking the two approaches.<br class=""></blockquote><div class=""><br class=""></div>I have written a proof-of-concept ffi implementation and tested its performance.</div><div class="">It has shown a ~25% performance decrease over the current representation.</div><div class="">The details are in the issue comments (<span class=""><a href="https://github.com/tarantool/tarantool/issues/692#issuecomment-505043929" class="">https://github.com/tarantool/tarantool/issues/692#issuecomment-505043929</a></span>)</div><div class="">I propose to leave existing implementation as is.<br class=""><br class=""></div><div class=""><br class=""></div><div class=""><br class=""></div><div class="">diff --git a/src/lua/decimal.c b/src/lua/decimal.c<br class="">index 9e2fd41b5..4dc0dff7d 100644<br class="">--- a/src/lua/decimal.c<br class="">+++ b/src/lua/decimal.c<br class="">@@ -227,10 +227,10 @@ LDECIMAL_CMPOP(lt, <);<br class=""> LDECIMAL_CMPOP(le, <=);<br class=""> <br class=""> static int<br class="">-ldecimal_tonumber(struct lua_State *L)<br class="">+ldecimal_new(struct lua_State *L)<br class=""> {<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>if (lua_gettop(L) < 1)<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>luaL_error(L, "Usage: decimal.tonumber(value)");<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>luaL_error(L, "Usage: decimal.new(value)");<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>lua_todecimal(L, 1);<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>decimal_t *lhs = lua_checkdecimal(L, 1);<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>decimal_t *res = lua_pushdecimal(L);<br class="">@@ -251,6 +251,28 @@ ldecimal_round(struct lua_State *L)<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>return 1;<br class=""> }<br class=""> <br class="">+static int<br class="">+ldecimal_scale(struct lua_State *L)<br class="">+{<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>if (lua_gettop(L) < 1)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>return luaL_error(L, "Usage: decimal.scale(decimal)");<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>decimal_t *lhs = lua_checkdecimal(L, 1);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>int scale = decimal_scale(lhs);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>lua_pushnumber(L, scale);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>return 1;<br class="">+}<br class="">+<br class="">+static int<br class="">+ldecimal_precision(struct lua_State *L)<br class="">+{<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>if (lua_gettop(L) < 1)<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>return luaL_error(L, "Usage: decimal.precisiion(decimal)");<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>decimal_t *lhs = lua_checkdecimal(L, 1);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>int precision = decimal_precision(lhs);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>lua_pushnumber(L, precision);<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>return 1;<br class="">+}<br class="">+<br class=""> static int<br class=""> ldecimal_tostring(struct lua_State *L)<br class=""> {<br class="">@@ -262,14 +284,6 @@ ldecimal_tostring(struct lua_State *L)<br class=""> }<br class=""> <br class=""> static const luaL_Reg ldecimal_mt[] = {<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>{"log10", ldecimal_log10},<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>{"ln", ldecimal_ln},<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>{"exp", ldecimal_exp},<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>{"sqrt", ldecimal_sqrt},<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>{"round", ldecimal_round},<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>{"minus", ldecimal_minus},<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>{"abs", ldecimal_abs},<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>{"tostring", ldecimal_tostring},<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>{"__unm", ldecimal_minus},<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>{"__add", ldecimal_add},<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>{"__sub", ldecimal_sub},<br class="">@@ -284,23 +298,16 @@ static const luaL_Reg ldecimal_mt[] = {<br class=""> };<br class=""> <br class=""> static const luaL_Reg ldecimal_lib[] = {<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>{"eq", ldecimal_eq},<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>{"lt", ldecimal_lt},<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>{"le", ldecimal_le},<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>{"add", ldecimal_add},<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>{"sub", ldecimal_sub},<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>{"mul", ldecimal_mul},<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>{"div", ldecimal_div},<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>{"log10", ldecimal_log10},<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>{"ln", ldecimal_ln},<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>{"pow", ldecimal_pow},<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>{"exp", ldecimal_exp},<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>{"sqrt", ldecimal_sqrt},<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>{"round", ldecimal_round},<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>{"minus", ldecimal_minus},<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>{"scale", ldecimal_scale},<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>{"precision", ldecimal_precision},<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>{"abs", ldecimal_abs},<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>{"tostring", ldecimal_tostring},<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span>{"tonumber", ldecimal_tonumber},<br class="">+<span class="Apple-tab-span" style="white-space:pre"> </span>{"new", ldecimal_new},<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>{NULL, NULL}<br class=""> };<br class=""> <br class="">diff --git a/test/app/decimal.result b/test/app/decimal.result<br class="">index 93c1c7557..22590a082 100644<br class="">--- a/test/app/decimal.result<br class="">+++ b/test/app/decimal.result<br class="">@@ -5,69 +5,69 @@ test_run = require('test_run').new()<br class=""> ---<br class=""> ...<br class=""> -- check various constructors<br class="">-decimal.tonumber('1234.5678')<br class="">+decimal.new('1234.5678')<br class=""> ---<br class=""> - '1234.5678'<br class=""> ...<br class="">-decimal.tonumber('1e6')<br class="">+decimal.new('1e6')<br class=""> ---<br class=""> - '1000000'<br class=""> ...<br class="">-decimal.tonumber('-6.234612e2')<br class="">+decimal.new('-6.234612e2')<br class=""> ---<br class=""> - '-623.4612'<br class=""> ...<br class="">-decimal.tonumber(tonumber64(2^63))<br class="">+decimal.new(tonumber64(2^63))<br class=""> ---<br class=""> - '9223372036854775808.000000000000000'<br class=""> ...<br class="">-decimal.tonumber(12345678ULL)<br class="">+decimal.new(12345678ULL)<br class=""> ---<br class=""> - '12345678'<br class=""> ...<br class="">-decimal.tonumber(-12345678LL)<br class="">+decimal.new(-12345678LL)<br class=""> ---<br class=""> - '-12345678'<br class=""> ...<br class="">-decimal.tonumber(1)<br class="">+decimal.new(1)<br class=""> ---<br class=""> - '1.000000000000000'<br class=""> ...<br class="">-decimal.tonumber(-1)<br class="">+decimal.new(-1)<br class=""> ---<br class=""> - '-1.000000000000000'<br class=""> ...<br class="">-decimal.tonumber(2^64)<br class="">+decimal.new(2^64)<br class=""> ---<br class=""> - '18446744073709551616.000000000000000'<br class=""> ...<br class="">-decimal.tonumber(2^(-20))<br class="">+decimal.new(2^(-20))<br class=""> ---<br class=""> - '0.000000953674316'<br class=""> ...<br class="">-a = decimal.tonumber('100')<br class="">+a = decimal.new('100')<br class=""> ---<br class=""> ...<br class="">-a:log10()<br class="">+decimal.log10(a)<br class=""> ---<br class=""> - '2'<br class=""> ...<br class="">-a:ln()<br class="">+decimal.ln(a)<br class=""> ---<br class=""> - '4.6051701859880913680359829093687284152'<br class=""> ...<br class=""> -- overflow, e^100 > 10^38<br class="">-a:exp()<br class="">+decimal.exp(a)<br class=""> ---<br class=""> - error: Operation failed<br class=""> ...<br class=""> -- e^87 < 10^38, no overflow, but rounds<br class=""> -- to 0 digits after the decimal point.<br class="">-decimal.tonumber(87):exp()<br class="">+decimal.exp(decimal.new(87))<br class=""> ---<br class=""> - '60760302250568721495223289381302760753'<br class=""> ...<br class="">-a:sqrt()<br class="">+decimal.sqrt(a)<br class=""> ---<br class=""> - '10'<br class=""> ...<br class="">@@ -75,26 +75,18 @@ a<br class=""> ---<br class=""> - '100'<br class=""> ...<br class="">-a = decimal.tonumber('-123.456')<br class="">+a = decimal.new('-123.456')<br class=""> ---<br class=""> ...<br class="">-a:round(2)<br class="">+decimal.round(a, 2)<br class=""> ---<br class=""> - '-123.46'<br class=""> ...<br class="">-a:round(1)<br class="">----<br class="">-- '-123.5'<br class="">-...<br class="">-a:round(0)<br class="">----<br class="">-- '-123'<br class="">-...<br class="">-a:abs()<br class="">+decimal.abs(a)<br class=""> ---<br class=""> - '123.456'<br class=""> ...<br class="">-a:tostring()<br class="">+a<br class=""> ---<br class=""> - '-123.456'<br class=""> ...<br class="">@@ -122,11 +114,11 @@ a ^ 2<br class=""> ---<br class=""> - '15241.383936'<br class=""> ...<br class="">-a:abs() ^ 0.5<br class="">+decimal.abs(a) ^ 0.5<br class=""> ---<br class=""> - '11.111075555498666484621494041182192341'<br class=""> ...<br class="">-a:abs() ^ 0.5 == a:abs():sqrt()<br class="">+decimal.abs(a) ^ 0.5 == decimal.sqrt(decimal.abs(a))<br class=""> ---<br class=""> - true<br class=""> ...<br class="">@@ -150,23 +142,3 @@ a == a<br class=""> ---<br class=""> - true<br class=""> ...<br class="">-decimal.tostring(a)<br class="">----<br class="">-- '-123.456'<br class="">-...<br class="">-a:tostring()<br class="">----<br class="">-- '-123.456'<br class="">-...<br class="">-decimal.abs(a)<br class="">----<br class="">-- '123.456'<br class="">-...<br class="">-decimal.minus(a)<br class="">----<br class="">-- '123.456'<br class="">-...<br class="">-decimal.round(a, 2)<br class="">----<br class="">-- '-123.46'<br class="">-...<br class="">diff --git a/test/app/decimal.test.lua b/test/app/decimal.test.lua<br class="">index 4751c1500..b101f07b6 100644<br class="">--- a/test/app/decimal.test.lua<br class="">+++ b/test/app/decimal.test.lua<br class="">@@ -2,49 +2,41 @@ decimal = require('decimal')<br class=""> test_run = require('test_run').new()<br class=""> <br class=""> -- check various constructors<br class="">-decimal.tonumber('1234.5678')<br class="">-decimal.tonumber('1e6')<br class="">-decimal.tonumber('-6.234612e2')<br class="">-decimal.tonumber(tonumber64(2^63))<br class="">-decimal.tonumber(12345678ULL)<br class="">-decimal.tonumber(-12345678LL)<br class="">-decimal.tonumber(1)<br class="">-decimal.tonumber(-1)<br class="">-decimal.tonumber(2^64)<br class="">-decimal.tonumber(2^(-20))<br class="">+decimal.new('1234.5678')<br class="">+decimal.new('1e6')<br class="">+decimal.new('-6.234612e2')<br class="">+decimal.new(tonumber64(2^63))<br class="">+decimal.new(12345678ULL)<br class="">+decimal.new(-12345678LL)<br class="">+decimal.new(1)<br class="">+decimal.new(-1)<br class="">+decimal.new(2^64)<br class="">+decimal.new(2^(-20))<br class=""> <br class="">-a = decimal.tonumber('100')<br class="">-a:log10()<br class="">-a:ln()<br class="">+a = decimal.new('100')<br class="">+decimal.log10(a)<br class="">+decimal.ln(a)<br class=""> -- overflow, e^100 > 10^38<br class="">-a:exp()<br class="">+decimal.exp(a)<br class=""> -- e^87 < 10^38, no overflow, but rounds<br class=""> -- to 0 digits after the decimal point.<br class="">-decimal.tonumber(87):exp()<br class="">-a:sqrt()<br class="">+decimal.exp(decimal.new(87))<br class="">+decimal.sqrt(a)<br class="">+a<br class="">+a = decimal.new('-123.456')<br class="">+decimal.round(a, 2)<br class="">+decimal.abs(a)<br class=""> a<br class="">-a = decimal.tonumber('-123.456')<br class="">-a:round(2)<br class="">-a:round(1)<br class="">-a:round(0)<br class="">-a:abs()<br class="">-a:tostring()<br class=""> -a<br class=""> a / 10<br class=""> a * 5<br class=""> a + 17<br class=""> a - 0.0001<br class=""> a ^ 2<br class="">-a:abs() ^ 0.5<br class="">-a:abs() ^ 0.5 == a:abs():sqrt()<br class="">+decimal.abs(a) ^ 0.5<br class="">+decimal.abs(a) ^ 0.5 == decimal.sqrt(decimal.abs(a))<br class=""> a - 2 < a - 1<br class=""> a + 1e-10 > a<br class=""> a <= a<br class=""> a >= a<br class=""> a == a<br class="">-<br class="">-decimal.tostring(a)<br class="">-a:tostring()<br class="">-decimal.abs(a)<br class="">-decimal.minus(a)<br class="">-decimal.round(a, 2)<br class=""><br class=""></div></body></html>