[Tarantool-patches] [PATCH 0/2] decimal: intrdoduce strtodec and IsInt

[Tarantool-patches] [PATCH 0/2] decimal: intrdoduce strtodec and IsInt

[Chris Sosnin]

branch: https://github.com/tarantool/tarantool/tree/ksosnin/gh-4415-dec-utils
related issue: https://github.com/tarantool/tarantool/issues/4415

Chris Sosnin (2):
  decNumber: bump new version
  decimal: introduce strtodec function

 src/lib/core/decimal.c   | 14 +++++++++++++-
 src/lib/core/decimal.h   | 15 +++++++++++++++
 test/unit/decimal.c      | 20 +++++++++++++++++++-
 test/unit/decimal.result | 18 +++++++++++++++++-
 third_party/decNumber    |  2 +-
 5 files changed, 65 insertions(+), 4 deletions(-)

-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 1/2] decNumber: bump new version

[Chris Sosnin]

---
 third_party/decNumber | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/third_party/decNumber b/third_party/decNumber
index 4f318a5d5..dba9d4273 160000
--- a/third_party/decNumber
+++ b/third_party/decNumber
@@ -1 +1 @@
-Subproject commit 4f318a5d5a1fce1f16f2d6a4e23aa5661647c779
+Subproject commit dba9d4273667484cb2b21e4d4175179908988dfa
-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 2/2] decimal: introduce strtodec function

[Chris Sosnin]

The behavior is similar with other strto* functions: parse the valid
beginning of the string and optionally store the pointer to the first
invalid character.

Needed for tarantool/tarantool#4415
---
 src/lib/core/decimal.c   | 14 +++++++++++++-
 src/lib/core/decimal.h   | 15 +++++++++++++++
 test/unit/decimal.c      | 20 +++++++++++++++++++-
 test/unit/decimal.result | 18 +++++++++++++++++-
 4 files changed, 64 insertions(+), 3 deletions(-)

diff --git a/src/lib/core/decimal.c b/src/lib/core/decimal.c
index 019c68338..e762266c5 100644
--- a/src/lib/core/decimal.c
+++ b/src/lib/core/decimal.c
@@ -112,7 +112,19 @@ decimal_zero(decimal_t *dec)
 decimal_t *
 decimal_from_string(decimal_t *dec, const char *str)
 {
-	decNumberFromString(dec, str, &decimal_context);
+	const char *end = decNumberFromString(dec, str, &decimal_context);
+	if (*end != '\0') {
+		decContextZeroStatus(&decimal_context);
+		return NULL;
+	}
+	return decimal_check_status(dec, &decimal_context);
+}
+
+decimal_t *
+strtodec(decimal_t *dec, const char *str, const char **endptr) {
+	const char *end = decNumberFromString(dec, str, &decimal_context);
+	if (endptr != NULL)
+		*endptr = end;
 	return decimal_check_status(dec, &decimal_context);
 }
 
diff --git a/src/lib/core/decimal.h b/src/lib/core/decimal.h
index e276344d2..992fbd267 100644
--- a/src/lib/core/decimal.h
+++ b/src/lib/core/decimal.h
@@ -77,6 +77,21 @@ decimal_zero(decimal_t *dec);
 decimal_t *
 decimal_from_string(decimal_t *dec, const char *str);
 
+/**
+ * Initialize a decimal with a value from the valid beginning
+ * of the string.
+ * If \a endptr is not NULL, store the address of the first
+ * invalid character in *endptr.
+ *
+ * If the number is less, than 10^DECIMAL_MAX_DIGITS,
+ * but has excess digits in fractional part, it will be rounded.
+ *
+ * @return NULL if string is invalid or
+ * the number is too big (>= 10^DECIMAL_MAX_DIGITS)
+ */
+decimal_t *
+strtodec(decimal_t *dec, const char *str, const char **endptr);
+
 /**
  * Initialize a decimal from double.
  *
diff --git a/test/unit/decimal.c b/test/unit/decimal.c
index 179723d02..cf74de313 100644
--- a/test/unit/decimal.c
+++ b/test/unit/decimal.c
@@ -73,6 +73,15 @@
 	is(decimal_##op(&b, &a), NULL, "decimal_"#op"("#stra") - error on wrong operands.");\
 })
 
+#define test_strtodec(str, end, expect) ({\
+	decimal_t dec;\
+	const char *endptr;\
+	is(strtodec(&dec, str, &endptr), expect(&dec), "strtodec("#str") "\
+							#expect);\
+	is(*endptr, end, "strtodec("#str") - expected end of valid string at "\
+			  #end);\
+})
+
 char buf[32];
 
 #define test_mpdec(str) ({\
@@ -317,7 +326,7 @@ test_mp_print(void)
 int
 main(void)
 {
-	plan(282);
+	plan(298);
 
 	dectest(314, 271, uint64, uint64_t);
 	dectest(65535, 23456, uint64, uint64_t);
@@ -384,5 +393,14 @@ main(void)
 	test_mp_decimal();
 	test_mp_print();
 
+	test_strtodec("15.e", 'e', success);
+	test_strtodec("15.e+", 'e', success);
+	test_strtodec(".0e-1", '\0', success);
+	test_strtodec("1.1003 2.2", ' ', success);
+	test_strtodec("cCC", 'c', failure);
+	test_strtodec(".e--", '.', failure);
+	test_strtodec("NaN", 'N', failure);
+	test_strtodec("inf", 'i', failure);
+
 	return check_plan();
 }
diff --git a/test/unit/decimal.result b/test/unit/decimal.result
index f11396df3..8be8ea122 100644
--- a/test/unit/decimal.result
+++ b/test/unit/decimal.result
@@ -1,4 +1,4 @@
-1..282
+1..298
 ok 1 - decimal(314)
 ok 2 - decimal(271)
 ok 3 - decimal(314) + decimal(271)
@@ -707,3 +707,19 @@ ok 281 - subtests
     ok 5 - correct mp_fprint result
 	*** test_mp_print: done ***
 ok 282 - subtests
+ok 283 - strtodec("15.e") success
+ok 284 - strtodec("15.e") - expected end of valid string at 'e'
+ok 285 - strtodec("15.e+") success
+ok 286 - strtodec("15.e+") - expected end of valid string at 'e'
+ok 287 - strtodec(".0e-1") success
+ok 288 - strtodec(".0e-1") - expected end of valid string at '\0'
+ok 289 - strtodec("1.1003 2.2") success
+ok 290 - strtodec("1.1003 2.2") - expected end of valid string at ' '
+ok 291 - strtodec("cCC") failure
+ok 292 - strtodec("cCC") - expected end of valid string at 'c'
+ok 293 - strtodec(".e--") failure
+ok 294 - strtodec(".e--") - expected end of valid string at '.'
+ok 295 - strtodec("NaN") failure
+ok 296 - strtodec("NaN") - expected end of valid string at 'N'
+ok 297 - strtodec("inf") failure
+ok 298 - strtodec("inf") - expected end of valid string at 'i'
-- 
2.21.1 (Apple Git-122.3)

Re: [Tarantool-patches] [PATCH 0/2] decimal: intrdoduce strtodec and IsInt

[Vladislav Shpilevoy]

The patchset LGTM except that I don't see a wrapper nor tests
for decNumberIsInt().

[Tarantool-patches] [PATCH] decimal: introduce decimal_is_int

[Chris Sosnin]

Thank you for the review!
For some reason I thought we don't need a wrapper for a function
that doesn't require context and the status check. So does it mean
I should also wrap isZero and similar calls in my SQL patchset?

I added the following commit with the wrapper and tests on the branch:
=====================================================================

This function will be used to determine, whether we can safely
convert to integer without an information loss.

Needed for #4415
---
 src/lib/core/decimal.c   |  6 ++++++
 src/lib/core/decimal.h   |  8 ++++++++
 test/unit/decimal.c      | 13 ++++++++++++-
 test/unit/decimal.result |  8 +++++++-
 4 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/src/lib/core/decimal.c b/src/lib/core/decimal.c
index e762266c5..95e809e62 100644
--- a/src/lib/core/decimal.c
+++ b/src/lib/core/decimal.c
@@ -109,6 +109,12 @@ decimal_zero(decimal_t *dec)
 	return dec;
 }
 
+bool
+decimal_is_int(decimal_t *dec)
+{
+	return decNumberIsInt(dec);
+}
+
 decimal_t *
 decimal_from_string(decimal_t *dec, const char *str)
 {
diff --git a/src/lib/core/decimal.h b/src/lib/core/decimal.h
index 992fbd267..a48ec723f 100644
--- a/src/lib/core/decimal.h
+++ b/src/lib/core/decimal.h
@@ -36,6 +36,7 @@
 #define DECNUMDIGITS DECIMAL_MAX_DIGITS
 #include "third_party/decNumber/decNumber.h"
 #include <stdint.h>
+#include <stdbool.h>
 
 #if defined(__cplusplus)
 extern "C" {
@@ -65,6 +66,13 @@ decimal_scale(const decimal_t *dec);
 decimal_t *
 decimal_zero(decimal_t *dec);
 
+/**
+ * @return true if the fractional part of the number is 0,
+ * false otherwise.
+ */
+bool
+decimal_is_int(decimal_t *dec);
+
 /**
  * Initialize a decimal with a value from the string.
  *
diff --git a/test/unit/decimal.c b/test/unit/decimal.c
index cf74de313..32694b88a 100644
--- a/test/unit/decimal.c
+++ b/test/unit/decimal.c
@@ -73,6 +73,13 @@
 	is(decimal_##op(&b, &a), NULL, "decimal_"#op"("#stra") - error on wrong operands.");\
 })
 
+#define dectest_is(op, stra, expect) ({\
+	decimal_t a;\
+	is(decimal_from_string(&a, #stra), &a, "decimal_from_string("#stra")");\
+	is(decimal_##op(&a), expect, "decimal_"#op"("#stra") - expected "\
+				      #expect);\
+})
+
 #define test_strtodec(str, end, expect) ({\
 	decimal_t dec;\
 	const char *endptr;\
@@ -326,7 +333,7 @@ test_mp_print(void)
 int
 main(void)
 {
-	plan(298);
+	plan(304);
 
 	dectest(314, 271, uint64, uint64_t);
 	dectest(65535, 23456, uint64, uint64_t);
@@ -402,5 +409,9 @@ main(void)
 	test_strtodec("NaN", 'N', failure);
 	test_strtodec("inf", 'i', failure);
 
+	dectest_is(is_int, 1, true);
+	dectest_is(is_int, 1.0000, true);
+	dectest_is(is_int, 1.0000001, false);
+
 	return check_plan();
 }
diff --git a/test/unit/decimal.result b/test/unit/decimal.result
index 8be8ea122..8f6de61a7 100644
--- a/test/unit/decimal.result
+++ b/test/unit/decimal.result
@@ -1,4 +1,4 @@
-1..298
+1..304
 ok 1 - decimal(314)
 ok 2 - decimal(271)
 ok 3 - decimal(314) + decimal(271)
@@ -723,3 +723,9 @@ ok 295 - strtodec("NaN") failure
 ok 296 - strtodec("NaN") - expected end of valid string at 'N'
 ok 297 - strtodec("inf") failure
 ok 298 - strtodec("inf") - expected end of valid string at 'i'
+ok 299 - decimal_from_string(1)
+ok 300 - decimal_is_int(1) - expected true
+ok 301 - decimal_from_string(1.0000)
+ok 302 - decimal_is_int(1.0000) - expected true
+ok 303 - decimal_from_string(1.0000001)
+ok 304 - decimal_is_int(1.0000001) - expected false
-- 
2.21.1 (Apple Git-122.3)

Re: [Tarantool-patches] [PATCH] decimal: introduce decimal_is_int

[Vladislav Shpilevoy]

On 26/06/2020 13:58, Chris Sosnin wrote:
> Thank you for the review!
> For some reason I thought we don't need a wrapper for a function
> that doesn't require context and the status check.

You need. Because decNumber code style is too different, even in
declarations. Also we use decimal_t type, not decNumber. Even though
they are the same for now. But this may change.

> So does it mean
> I should also wrap isZero and similar calls in my SQL patchset?

Yes. All decimal functions should be decimal_*(). Not decNumber*(). At
least this is how I understand it. Sergey may have different thoughts.

The patchset LGTM.

Re: [Tarantool-patches] [PATCH] decimal: introduce decimal_is_int

[Chris Sosnin]

Thank you for your answers, this will be helpful.

> On 28 Jun 2020, at 19:53, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
> 
> On 26/06/2020 13:58, Chris Sosnin wrote:
>> Thank you for the review!
>> For some reason I thought we don't need a wrapper for a function
>> that doesn't require context and the status check.
> 
> You need. Because decNumber code style is too different, even in
> declarations. Also we use decimal_t type, not decNumber. Even though
> they are the same for now. But this may change.
> 
>> So does it mean
>> I should also wrap isZero and similar calls in my SQL patchset?
> 
> Yes. All decimal functions should be decimal_*(). Not decNumber*(). At
> least this is how I understand it. Sergey may have different thoughts.
> 
> The patchset LGTM.

decNumber patchset has 2 LGTMs, and wrappers for the core have one, can we push it?
I could rebase on it and continue finalizing decimals.

Re: [Tarantool-patches] [PATCH 0/2] decimal: intrdoduce strtodec and IsInt

[Vladislav Shpilevoy]

Pushed to master.

On 25/06/2020 16:23, Chris Sosnin wrote:
> branch: https://github.com/tarantool/tarantool/tree/ksosnin/gh-4415-dec-utils
> related issue: https://github.com/tarantool/tarantool/issues/4415
> 
> Chris Sosnin (2):
>   decNumber: bump new version
>   decimal: introduce strtodec function
> 
>  src/lib/core/decimal.c   | 14 +++++++++++++-
>  src/lib/core/decimal.h   | 15 +++++++++++++++
>  test/unit/decimal.c      | 20 +++++++++++++++++++-
>  test/unit/decimal.result | 18 +++++++++++++++++-
>  third_party/decNumber    |  2 +-
>  5 files changed, 65 insertions(+), 4 deletions(-)