[Tarantool-patches] [PATCH 0/2] update decNumber for SQL

[Tarantool-patches] [PATCH 0/2] update decNumber for SQL

[Chris Sosnin]

branch: https://github.com/tarantool/decNumber/tree/ksosnin/utilities-for-sql
issue: https://github.com/tarantool/tarantool/issues/4415

Chris Sosnin (2):
  Allow leading and trailing whitespaces in FromString
  Add IsWhole method for checking the fractional part of a number

 decNumber.c | 74 ++++++++++++++++++++++++++++++++++++++++++-----------
 decNumber.h |  1 +
 2 files changed, 60 insertions(+), 15 deletions(-)

-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 1/2] Allow leading and trailing whitespaces in FromString

[Chris Sosnin]

This is needed for SQL, since these whitespaces are allowed in
conversions from string.

Needed for tarantool/tarantool#4415
---
 decNumber.c | 44 +++++++++++++++++++++++++++++---------------
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/decNumber.c b/decNumber.c
index 85deb13..7c53bc3 100644
--- a/decNumber.c
+++ b/decNumber.c
@@ -555,9 +555,9 @@ decNumber * decNumberFromString(decNumber *dn, const char chars[],
   Unit  *allocres=NULL;            // -> allocated result, iff allocated
   Int   d=0;                       // count of digits found in decimal part
   const char *dotchar=NULL;        // where dot was found
-  const char *cfirst=chars;        // -> first character of decimal part
+  const char *cfirst=NULL;         // -> first character of decimal part
   const char *last=NULL;           // -> last digit of decimal part
-  const char *c;                   // work
+  const char *c, *p;               // work
   Unit  *up;                       // ..
   #if DECDPUN>1
   Int   cut, out;                  // ..
@@ -571,6 +571,9 @@ decNumber * decNumberFromString(decNumber *dn, const char chars[],
   #endif
 
   do {                             // status & malloc protection
+    for (; *chars==' '; ++chars);  // skip leading whitespaces
+    cfirst=chars;
+
     for (c=chars;; c++) {          // -> input character
       if (*c>='0' && *c<='9') {    // test for Arabic digit
         last=c;
@@ -590,13 +593,14 @@ decNumber * decNumberFromString(decNumber *dn, const char chars[],
           cfirst++;
           continue;}
         }
+      for (p=c; *p==' '; ++p);
       // *c is not a digit, or a valid +, -, or '.'
       break;
       } // c
 
     if (last==NULL) {              // no digits yet
       status=DEC_Conversion_syntax;// assume the worst
-      if (*c=='\0') break;         // and no more to come...
+      if (*p=='\0') break;         // and no more to come...
       #if DECSUBSET
       // if subset then infinities and NaNs are not allowed
       if (!set->extended) break;   // hopeless
@@ -626,7 +630,8 @@ decNumber * decNumberFromString(decNumber *dn, const char chars[],
       // now either nothing, or nnnn payload, expected
       // -> start of integer and skip leading 0s [including plain 0]
       for (cfirst=c; *cfirst=='0';) cfirst++;
-      if (*cfirst=='\0') {         // "NaN" or "sNaN", maybe with all 0s
+      for (p=cfirst; *p==' '; ++p);
+      if (*p=='\0') {              // "NaN" or "sNaN", maybe with all 0s
         status=0;                  // it's good
         break;                     // ..
         }
@@ -635,7 +640,8 @@ decNumber * decNumberFromString(decNumber *dn, const char chars[],
         if (*c<'0' || *c>'9') break; // test for Arabic digit
         last=c;
         }
-      if (*c!='\0') break;         // not all digits
+      for (p=c; *p==' '; ++p);
+      if (*p!='\0') break;         // not all digits
       if (d>set->digits-1) {
         // [NB: payload in a decNumber can be full length unless
         // clamped, in which case can only be digits-1]
@@ -647,7 +653,7 @@ decNumber * decNumberFromString(decNumber *dn, const char chars[],
       bits=dn->bits;               // for copy-back
       } // last==NULL
 
-     else if (*c!='\0') {          // more to process...
+     else if (p==c && *p!='\0') {  // more to process...
       // had some digits; exponent is only valid sequence now
       Flag nege;                   // 1=negative exponent
       const char *firstexp;        // -> first significant exponent digit
@@ -659,16 +665,18 @@ decNumber * decNumberFromString(decNumber *dn, const char chars[],
       c++;                         // to (possible) sign
       if (*c=='-') {nege=1; c++;}
        else if (*c=='+') c++;
-      if (*c=='\0') break;
-
-      for (; *c=='0' && *(c+1)!='\0';) c++;  // strip insignificant zeros
+      for (p=c; *p==' '; ++p);
+      if (*p=='\0') break;
+      // strip insignificant zeros
+      for (; *c=='0' && *(c+1)!='\0' && *(c+1)!=' ';) c++;
       firstexp=c;                            // save exponent digit place
       for (; ;c++) {
         if (*c<'0' || *c>'9') break;         // not a digit
         exponent=X10(exponent)+(Int)*c-(Int)'0';
         } // c
       // if not now on a '\0', *c must not be a digit
-      if (*c!='\0') break;
+      for (p=c; *p==' '; ++p);
+      if (*p!='\0') break;
 
       // (this next test must be after the syntax checks)
       // if it was too long the exponent may have wrapped, so check
@@ -681,6 +689,11 @@ decNumber * decNumberFromString(decNumber *dn, const char chars[],
       status=0;                         // is OK
       } // stuff after digits
 
+     else if (*p!='\0') {    // whitespaces in the middle
+       status=DEC_Conversion_syntax;
+       break;
+     }
+
     // Here when whole string has been inspected; syntax is good
     // cfirst->first digit (never dot), last->last digit (ditto)
 
@@ -7739,12 +7752,13 @@ static decNumber *decDecap(decNumber *dn, Int drop) {
 /*   if (decBiStr(test, "mike", "MIKE")) ...                          */
 /* ------------------------------------------------------------------ */
 static Flag decBiStr(const char *targ, const char *str1, const char *str2) {
-  for (;;targ++, str1++, str2++) {
+  const char *end=strchr(targ, ' ');
+  if (end==NULL)
+    end=targ+strlen(targ);
+  for (;targ<end; targ++, str1++, str2++) {
     if (*targ!=*str1 && *targ!=*str2) return 0;
-    // *targ has a match in one (or both, if terminator)
-    if (*targ=='\0') break;
-    } // forever
-  return 1;
+    } // for
+  return *str1=='\0';
   } // decBiStr
 
 /* ------------------------------------------------------------------ */
-- 
2.21.1 (Apple Git-122.3)

Re: [Tarantool-patches] [PATCH 1/2] Allow leading and trailing whitespaces in FromString

[Serge Petrenko]

05.06.2020 18:41, Chris Sosnin пишет:
> This is needed for SQL, since these whitespaces are allowed in
> conversions from string.
>
> Needed for tarantool/tarantool#4415

Hi! Thanks for the patch!

See one comment below.

> ---
>   decNumber.c | 44 +++++++++++++++++++++++++++++---------------
>   1 file changed, 29 insertions(+), 15 deletions(-)
>
> diff --git a/decNumber.c b/decNumber.c
> index 85deb13..7c53bc3 100644
> --- a/decNumber.c
> +++ b/decNumber.c
> @@ -555,9 +555,9 @@ decNumber * decNumberFromString(decNumber *dn, const char chars[],
>     Unit  *allocres=NULL;            // -> allocated result, iff allocated
>     Int   d=0;                       // count of digits found in decimal part
>     const char *dotchar=NULL;        // where dot was found
> -  const char *cfirst=chars;        // -> first character of decimal part
> +  const char *cfirst=NULL;         // -> first character of decimal part
>     const char *last=NULL;           // -> last digit of decimal part
> -  const char *c;                   // work
> +  const char *c, *p;               // work
>     Unit  *up;                       // ..
>     #if DECDPUN>1
>     Int   cut, out;                  // ..
> @@ -571,6 +571,9 @@ decNumber * decNumberFromString(decNumber *dn, const char chars[],
>     #endif
>   
>     do {                             // status & malloc protection
> +    for (; *chars==' '; ++chars);  // skip leading whitespaces
> +    cfirst=chars;
> +
>       for (c=chars;; c++) {          // -> input character
>         if (*c>='0' && *c<='9') {    // test for Arabic digit
>           last=c;
> @@ -590,13 +593,14 @@ decNumber * decNumberFromString(decNumber *dn, const char chars[],
>             cfirst++;
>             continue;}
>           }
> +      for (p=c; *p==' '; ++p);
Looks like this `for` loop should go below.
>         // *c is not a digit, or a valid +, -, or '.'
>         break;
>         } // c
>   

Here, ^ (This guy's formatting is confusing as hell, I know)

>       if (last==NULL) {              // no digits yet
>         status=DEC_Conversion_syntax;// assume the worst
> -      if (*c=='\0') break;         // and no more to come...
> +      if (*p=='\0') break;         // and no more to come...
>         #if DECSUBSET
>         // if subset then infinities and NaNs are not allowed
>         if (!set->extended) break;   // hopeless
> @@ -626,7 +630,8 @@ decNumber * decNumberFromString(decNumber *dn, const char chars[],
>         // now either nothing, or nnnn payload, expected
>         // -> start of integer and skip leading 0s [including plain 0]
>         for (cfirst=c; *cfirst=='0';) cfirst++;
> -      if (*cfirst=='\0') {         // "NaN" or "sNaN", maybe with all 0s
> +      for (p=cfirst; *p==' '; ++p);
> +      if (*p=='\0') {              // "NaN" or "sNaN", maybe with all 0s
>           status=0;                  // it's good
>           break;                     // ..
>           }
> @@ -635,7 +640,8 @@ decNumber * decNumberFromString(decNumber *dn, const char chars[],
>           if (*c<'0' || *c>'9') break; // test for Arabic digit
>           last=c;
>           }
> -      if (*c!='\0') break;         // not all digits
> +      for (p=c; *p==' '; ++p);
> +      if (*p!='\0') break;         // not all digits
>         if (d>set->digits-1) {
>           // [NB: payload in a decNumber can be full length unless
>           // clamped, in which case can only be digits-1]
> @@ -647,7 +653,7 @@ decNumber * decNumberFromString(decNumber *dn, const char chars[],
>         bits=dn->bits;               // for copy-back
>         } // last==NULL
>   
> -     else if (*c!='\0') {          // more to process...
> +     else if (p==c && *p!='\0') {  // more to process...
>         // had some digits; exponent is only valid sequence now
>         Flag nege;                   // 1=negative exponent
>         const char *firstexp;        // -> first significant exponent digit
> @@ -659,16 +665,18 @@ decNumber * decNumberFromString(decNumber *dn, const char chars[],
>         c++;                         // to (possible) sign
>         if (*c=='-') {nege=1; c++;}
>          else if (*c=='+') c++;
> -      if (*c=='\0') break;
> -
> -      for (; *c=='0' && *(c+1)!='\0';) c++;  // strip insignificant zeros
> +      for (p=c; *p==' '; ++p);
> +      if (*p=='\0') break;
> +      // strip insignificant zeros
> +      for (; *c=='0' && *(c+1)!='\0' && *(c+1)!=' ';) c++;
>         firstexp=c;                            // save exponent digit place
>         for (; ;c++) {
>           if (*c<'0' || *c>'9') break;         // not a digit
>           exponent=X10(exponent)+(Int)*c-(Int)'0';
>           } // c
>         // if not now on a '\0', *c must not be a digit
> -      if (*c!='\0') break;
> +      for (p=c; *p==' '; ++p);
> +      if (*p!='\0') break;
>   
>         // (this next test must be after the syntax checks)
>         // if it was too long the exponent may have wrapped, so check
> @@ -681,6 +689,11 @@ decNumber * decNumberFromString(decNumber *dn, const char chars[],
>         status=0;                         // is OK
>         } // stuff after digits
>   
> +     else if (*p!='\0') {    // whitespaces in the middle
> +       status=DEC_Conversion_syntax;
> +       break;
> +     }
> +
>       // Here when whole string has been inspected; syntax is good
>       // cfirst->first digit (never dot), last->last digit (ditto)
>   
> @@ -7739,12 +7752,13 @@ static decNumber *decDecap(decNumber *dn, Int drop) {
>   /*   if (decBiStr(test, "mike", "MIKE")) ...                          */
>   /* ------------------------------------------------------------------ */
>   static Flag decBiStr(const char *targ, const char *str1, const char *str2) {
> -  for (;;targ++, str1++, str2++) {
> +  const char *end=strchr(targ, ' ');
> +  if (end==NULL)
> +    end=targ+strlen(targ);
> +  for (;targ<end; targ++, str1++, str2++) {
>       if (*targ!=*str1 && *targ!=*str2) return 0;
> -    // *targ has a match in one (or both, if terminator)
> -    if (*targ=='\0') break;
> -    } // forever
> -  return 1;
> +    } // for
> +  return *str1=='\0';
>     } // decBiStr
>   
>   /* ------------------------------------------------------------------ */

-- 
Serge Petrenko

Re: [Tarantool-patches] [PATCH 1/2] Allow leading and trailing whitespaces in FromString

[Chris Sosnin]

Hi! Thank you for the review!

> On 6 Jun 2020, at 16:48, Serge Petrenko <sergepetrenko@tarantool.org> wrote:
> 
> 
> 05.06.2020 18:41, Chris Sosnin пишет:
>> This is needed for SQL, since these whitespaces are allowed in
>> conversions from string.
>> 
>> Needed for tarantool/tarantool#4415
> 
> Hi! Thanks for the patch!
> 
> See one comment below.
> 
>> ---
>>  decNumber.c | 44 +++++++++++++++++++++++++++++---------------
>>  1 file changed, 29 insertions(+), 15 deletions(-)
>> 
>> diff --git a/decNumber.c b/decNumber.c
>> index 85deb13..7c53bc3 100644
>> --- a/decNumber.c
>> +++ b/decNumber.c
>> @@ -555,9 +555,9 @@ decNumber * decNumberFromString(decNumber *dn, const char chars[],
>>    Unit  *allocres=NULL;            // -> allocated result, iff allocated
>>    Int   d=0;                       // count of digits found in decimal part
>>    const char *dotchar=NULL;        // where dot was found
>> -  const char *cfirst=chars;        // -> first character of decimal part
>> +  const char *cfirst=NULL;         // -> first character of decimal part
>>    const char *last=NULL;           // -> last digit of decimal part
>> -  const char *c;                   // work
>> +  const char *c, *p;               // work
>>    Unit  *up;                       // ..
>>    #if DECDPUN>1
>>    Int   cut, out;                  // ..
>> @@ -571,6 +571,9 @@ decNumber * decNumberFromString(decNumber *dn, const char chars[],
>>    #endif
>>      do {                             // status & malloc protection
>> +    for (; *chars==' '; ++chars);  // skip leading whitespaces
>> +    cfirst=chars;
>> +
>>      for (c=chars;; c++) {          // -> input character
>>        if (*c>='0' && *c<='9') {    // test for Arabic digit
>>          last=c;
>> @@ -590,13 +593,14 @@ decNumber * decNumberFromString(decNumber *dn, const char chars[],
>>            cfirst++;
>>            continue;}
>>          }
>> +      for (p=c; *p==' '; ++p);
> Looks like this `for` loop should go below.
>>        // *c is not a digit, or a valid +, -, or '.'
>>        break;
>>        } // c
>>  
> 
> Here, ^ (This guy's formatting is confusing as hell, I know)

You are right, thank you for the catch!
I applied the change on the branch.

> 
>>      if (last==NULL) {              // no digits yet
>>        status=DEC_Conversion_syntax;// assume the worst
>> -      if (*c=='\0') break;         // and no more to come...
>> +      if (*p=='\0') break;         // and no more to come...
>>        #if DECSUBSET
>>        // if subset then infinities and NaNs are not allowed
>>        if (!set->extended) break;   // hopeless
>> @@ -626,7 +630,8 @@ decNumber * decNumberFromString(decNumber *dn, const char chars[],
>>        // now either nothing, or nnnn payload, expected
>>        // -> start of integer and skip leading 0s [including plain 0]
>>        for (cfirst=c; *cfirst=='0';) cfirst++;
>> -      if (*cfirst=='\0') {         // "NaN" or "sNaN", maybe with all 0s
>> +      for (p=cfirst; *p==' '; ++p);
>> +      if (*p=='\0') {              // "NaN" or "sNaN", maybe with all 0s
>>          status=0;                  // it's good
>>          break;                     // ..
>>          }
>> @@ -635,7 +640,8 @@ decNumber * decNumberFromString(decNumber *dn, const char chars[],
>>          if (*c<'0' || *c>'9') break; // test for Arabic digit
>>          last=c;
>>          }
>> -      if (*c!='\0') break;         // not all digits
>> +      for (p=c; *p==' '; ++p);
>> +      if (*p!='\0') break;         // not all digits
>>        if (d>set->digits-1) {
>>          // [NB: payload in a decNumber can be full length unless
>>          // clamped, in which case can only be digits-1]
>> @@ -647,7 +653,7 @@ decNumber * decNumberFromString(decNumber *dn, const char chars[],
>>        bits=dn->bits;               // for copy-back
>>        } // last==NULL
>>  -     else if (*c!='\0') {          // more to process...
>> +     else if (p==c && *p!='\0') {  // more to process...
>>        // had some digits; exponent is only valid sequence now
>>        Flag nege;                   // 1=negative exponent
>>        const char *firstexp;        // -> first significant exponent digit
>> @@ -659,16 +665,18 @@ decNumber * decNumberFromString(decNumber *dn, const char chars[],
>>        c++;                         // to (possible) sign
>>        if (*c=='-') {nege=1; c++;}
>>         else if (*c=='+') c++;
>> -      if (*c=='\0') break;
>> -
>> -      for (; *c=='0' && *(c+1)!='\0';) c++;  // strip insignificant zeros
>> +      for (p=c; *p==' '; ++p);
>> +      if (*p=='\0') break;
>> +      // strip insignificant zeros
>> +      for (; *c=='0' && *(c+1)!='\0' && *(c+1)!=' ';) c++;
>>        firstexp=c;                            // save exponent digit place
>>        for (; ;c++) {
>>          if (*c<'0' || *c>'9') break;         // not a digit
>>          exponent=X10(exponent)+(Int)*c-(Int)'0';
>>          } // c
>>        // if not now on a '\0', *c must not be a digit
>> -      if (*c!='\0') break;
>> +      for (p=c; *p==' '; ++p);
>> +      if (*p!='\0') break;
>>          // (this next test must be after the syntax checks)
>>        // if it was too long the exponent may have wrapped, so check
>> @@ -681,6 +689,11 @@ decNumber * decNumberFromString(decNumber *dn, const char chars[],
>>        status=0;                         // is OK
>>        } // stuff after digits
>>  +     else if (*p!='\0') {    // whitespaces in the middle
>> +       status=DEC_Conversion_syntax;
>> +       break;
>> +     }
>> +
>>      // Here when whole string has been inspected; syntax is good
>>      // cfirst->first digit (never dot), last->last digit (ditto)
>>  @@ -7739,12 +7752,13 @@ static decNumber *decDecap(decNumber *dn, Int drop) {
>>  /*   if (decBiStr(test, "mike", "MIKE")) ...                          */
>>  /* ------------------------------------------------------------------ */
>>  static Flag decBiStr(const char *targ, const char *str1, const char *str2) {
>> -  for (;;targ++, str1++, str2++) {
>> +  const char *end=strchr(targ, ' ');
>> +  if (end==NULL)
>> +    end=targ+strlen(targ);
>> +  for (;targ<end; targ++, str1++, str2++) {
>>      if (*targ!=*str1 && *targ!=*str2) return 0;
>> -    // *targ has a match in one (or both, if terminator)
>> -    if (*targ=='\0') break;
>> -    } // forever
>> -  return 1;
>> +    } // for
>> +  return *str1=='\0';
>>    } // decBiStr
>>    /* ------------------------------------------------------------------ */
> 
> -- 
> Serge Petrenko

Re: [Tarantool-patches] [PATCH 1/2] Allow leading and trailing whitespaces in FromString

[Vladislav Shpilevoy]

Hi! Thanks for the patch!

I am afraid this may be a dead end. You didn't check for tabs, new lines,
\r, and whatever else can be treated as a whitespace. Moreover, for some other
code it may be necessary not to skip whitespaces. Why can't you trim whitespace
symbols in SQL code? sql_atoi64(), for example, trims them, before calling
strtoll()/strtoull().

To trim right spaces you could make this function return a position where
did it stop, so as to ensure that all the rest is whitespaces, in SQL code.
That also would be consistent with strtoll()/strtoull().

Re: [Tarantool-patches] [PATCH 1/2] Allow leading and trailing whitespaces in FromString

[Chris Sosnin]

Hi! Thank you for the review!

> On 11 Jun 2020, at 20:06, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
> 
> Hi! Thanks for the patch!
> 
> I am afraid this may be a dead end. You didn't check for tabs, new lines,
> \r, and whatever else can be treated as a whitespace. Moreover, for some other
> code it may be necessary not to skip whitespaces. Why can't you trim whitespace
> symbols in SQL code? sql_atoi64(), for example, trims them, before calling
> strtoll()/strtoull().

I made this change here to unify the behavior, but perhaps this makes sense only in SQL?
In this case I can make a wrapper similar to sql_atoi64() you mentioned. And if I understand
correctly, I should use isspace instead of char comparison.

> 
> To trim right spaces you could make this function return a position where
> did it stop, so as to ensure that all the rest is whitespaces, in SQL code.
> That also would be consistent with strtoll()/strtoull().

I like this idea, will work on it in v2.

[Tarantool-patches] [PATCH 2/2] Add IsWhole method for checking the fractional part of a number

[Chris Sosnin]

Currently there is no efficient way to do this.

Needed for tarantool/tarantool#4415
---
 decNumber.c | 30 ++++++++++++++++++++++++++++++
 decNumber.h |  1 +
 2 files changed, 31 insertions(+)

diff --git a/decNumber.c b/decNumber.c
index 7c53bc3..f4ad927 100644
--- a/decNumber.c
+++ b/decNumber.c
@@ -501,6 +501,36 @@ uLong decNumberToUInt64(const decNumber *dn, decContext *set) {
   return 0;
   }  // decNumberToUInt64
 
+Flag decNumberIsWhole(const decNumber *dn) {
+  const Unit *up=dn->lsu;
+  if (dn->exponent>=0) {
+    return 1;
+    }
+   else {
+    Int count=-dn->exponent;
+    // spin up whole units until reach the Unit with the unit digit
+    for (; count>=DECDPUN; up++) {
+      if (*up!=0) return 0;
+      count-=DECDPUN;
+      }
+    if (count==0) return 1;             // [a multiple of DECDPUN]
+     else {                             // [not multiple of DECDPUN]
+      Int rem, theInt=0;                          // work
+      // slice off fraction digits and check for non-zero
+      #if DECDPUN<=4
+        theInt=QUOT10(*up, count);
+        rem=*up-theInt*powers[count];
+      #else
+        rem=*up%powers[count];          // slice off discards
+        theInt=*up/powers[count];
+      #endif
+      if (rem!=0) return 0;
+      up++;
+      }
+    }
+    return 1;
+  }  // decNumberIsWhole
+
 /* ------------------------------------------------------------------ */
 /* to-scientific-string -- conversion to numeric string               */
 /* to-engineering-string -- conversion to numeric string              */
diff --git a/decNumber.h b/decNumber.h
index 85a3f3f..71ab27f 100644
--- a/decNumber.h
+++ b/decNumber.h
@@ -169,6 +169,7 @@
   decNumber  * decNumberTrim(decNumber *);
   const char * decNumberVersion(void);
   decNumber  * decNumberZero(decNumber *);
+  uint8_t      decNumberIsWhole(const decNumber *dn);
 
   /* Functions for testing decNumbers (normality depends on context)  */
   int32_t decNumberIsNormal(const decNumber *, decContext *);
-- 
2.21.1 (Apple Git-122.3)

Re: [Tarantool-patches] [PATCH 2/2] Add IsWhole method for checking the fractional part of a number

[Serge Petrenko]

05.06.2020 18:41, Chris Sosnin пишет:
> Currently there is no efficient way to do this.
>
> Needed for tarantool/tarantool#4415
Thanks for the patch!
> ---
>   decNumber.c | 30 ++++++++++++++++++++++++++++++
>   decNumber.h |  1 +
>   2 files changed, 31 insertions(+)
>
> diff --git a/decNumber.c b/decNumber.c
> index 7c53bc3..f4ad927 100644
> --- a/decNumber.c
> +++ b/decNumber.c
> @@ -501,6 +501,36 @@ uLong decNumberToUInt64(const decNumber *dn, decContext *set) {
>     return 0;
>     }  // decNumberToUInt64
>   
> +Flag decNumberIsWhole(const decNumber *dn) {
> +  const Unit *up=dn->lsu;
> +  if (dn->exponent>=0) {
> +    return 1;
> +    }
> +   else {
> +    Int count=-dn->exponent;
> +    // spin up whole units until reach the Unit with the unit digit
> +    for (; count>=DECDPUN; up++) {
> +      if (*up!=0) return 0;
> +      count-=DECDPUN;
> +      }
> +    if (count==0) return 1;             // [a multiple of DECDPUN]
> +     else {                             // [not multiple of DECDPUN]
> +      Int rem, theInt=0;                          // work
> +      // slice off fraction digits and check for non-zero
> +      #if DECDPUN<=4
> +        theInt=QUOT10(*up, count);
> +        rem=*up-theInt*powers[count];
> +      #else
> +        rem=*up%powers[count];          // slice off discards
> +        theInt=*up/powers[count];
> +      #endif
> +      if (rem!=0) return 0;
> +      up++;
> +      }
> +    }
> +    return 1;
> +  }  // decNumberIsWhole
> +

Consider this  diff:

diff --git a/decNumber.c b/decNumber.c
index f4ad927..acc3fb1 100644
--- a/decNumber.c
+++ b/decNumber.c
@@ -515,17 +515,14 @@ Flag decNumberIsWhole(const decNumber *dn) {
        }
      if (count==0) return 1;             // [a multiple of DECDPUN]
       else {                             // [not multiple of DECDPUN]
-      Int rem, theInt=0;                          // work
+      Int rem;                          // work
        // slice off fraction digits and check for non-zero
        #if DECDPUN<=4
-        theInt=QUOT10(*up, count);
-        rem=*up-theInt*powers[count];
+        rem=*up-QUOT10(*up, count)*powers[count];
        #else
          rem=*up%powers[count];          // slice off discards
-        theInt=*up/powers[count];
        #endif
        if (rem!=0) return 0;
-      up++;
        }
      }
      return 1;

>   /* ------------------------------------------------------------------ */
>   /* to-scientific-string -- conversion to numeric string               */
>   /* to-engineering-string -- conversion to numeric string              */
> diff --git a/decNumber.h b/decNumber.h
> index 85a3f3f..71ab27f 100644
> --- a/decNumber.h
> +++ b/decNumber.h
> @@ -169,6 +169,7 @@
>     decNumber  * decNumberTrim(decNumber *);
>     const char * decNumberVersion(void);
>     decNumber  * decNumberZero(decNumber *);
> +  uint8_t      decNumberIsWhole(const decNumber *dn);
>   
>     /* Functions for testing decNumbers (normality depends on context)  */
>     int32_t decNumberIsNormal(const decNumber *, decContext *);

-- 
Serge Petrenko

Re: [Tarantool-patches] [PATCH 2/2] Add IsWhole method for checking the fractional part of a number

[Chris Sosnin]

Thank you for the review!

> On 6 Jun 2020, at 17:17, Serge Petrenko <sergepetrenko@tarantool.org> wrote:
> 
> 05.06.2020 18:41, Chris Sosnin пишет:
>> Currently there is no efficient way to do this.
>> 
>> Needed for tarantool/tarantool#4415
> Thanks for the patch!
>> ---
>>  decNumber.c | 30 ++++++++++++++++++++++++++++++
>>  decNumber.h |  1 +
>>  2 files changed, 31 insertions(+)
>> 
>> diff --git a/decNumber.c b/decNumber.c
>> index 7c53bc3..f4ad927 100644
>> --- a/decNumber.c
>> +++ b/decNumber.c
>> @@ -501,6 +501,36 @@ uLong decNumberToUInt64(const decNumber *dn, decContext *set) {
>>    return 0;
>>    }  // decNumberToUInt64
>>  +Flag decNumberIsWhole(const decNumber *dn) {
>> +  const Unit *up=dn->lsu;
>> +  if (dn->exponent>=0) {
>> +    return 1;
>> +    }
>> +   else {
>> +    Int count=-dn->exponent;
>> +    // spin up whole units until reach the Unit with the unit digit
>> +    for (; count>=DECDPUN; up++) {
>> +      if (*up!=0) return 0;
>> +      count-=DECDPUN;
>> +      }
>> +    if (count==0) return 1;             // [a multiple of DECDPUN]
>> +     else {                             // [not multiple of DECDPUN]
>> +      Int rem, theInt=0;                          // work
>> +      // slice off fraction digits and check for non-zero
>> +      #if DECDPUN<=4
>> +        theInt=QUOT10(*up, count);
>> +        rem=*up-theInt*powers[count];
>> +      #else
>> +        rem=*up%powers[count];          // slice off discards
>> +        theInt=*up/powers[count];
>> +      #endif
>> +      if (rem!=0) return 0;
>> +      up++;
>> +      }
>> +    }
>> +    return 1;
>> +  }  // decNumberIsWhole
>> +
> 
> Consider this  diff:
> 
> diff --git a/decNumber.c b/decNumber.c
> index f4ad927..acc3fb1 100644
> --- a/decNumber.c
> +++ b/decNumber.c
> @@ -515,17 +515,14 @@ Flag decNumberIsWhole(const decNumber *dn) {
>        }
>      if (count==0) return 1;             // [a multiple of DECDPUN]
>       else {                             // [not multiple of DECDPUN]
> -      Int rem, theInt=0;                          // work
> +      Int rem;                          // work
>        // slice off fraction digits and check for non-zero
>        #if DECDPUN<=4
> -        theInt=QUOT10(*up, count);
> -        rem=*up-theInt*powers[count];
> +        rem=*up-QUOT10(*up, count)*powers[count];
>        #else
>          rem=*up%powers[count];          // slice off discards
> -        theInt=*up/powers[count];
>        #endif
>        if (rem!=0) return 0;
> -      up++;
>        }
>      }
>      return 1;

I agree, looks much better, thanks!
Applied on the branch.

> 
>>  /* ------------------------------------------------------------------ */
>>  /* to-scientific-string -- conversion to numeric string               */
>>  /* to-engineering-string -- conversion to numeric string              */
>> diff --git a/decNumber.h b/decNumber.h
>> index 85a3f3f..71ab27f 100644
>> --- a/decNumber.h
>> +++ b/decNumber.h
>> @@ -169,6 +169,7 @@
>>    decNumber  * decNumberTrim(decNumber *);
>>    const char * decNumberVersion(void);
>>    decNumber  * decNumberZero(decNumber *);
>> +  uint8_t      decNumberIsWhole(const decNumber *dn);
>>      /* Functions for testing decNumbers (normality depends on context)  */
>>    int32_t decNumberIsNormal(const decNumber *, decContext *);
> 
> -- 
> Serge Petrenko

Re: [Tarantool-patches] [PATCH 2/2] Add IsWhole method for checking the fractional part of a number

[Serge Petrenko]

07.06.2020 15:40, Chris Sosnin пишет:
> Thank you for the review!
>
>> On 6 Jun 2020, at 17:17, Serge Petrenko <sergepetrenko@tarantool.org> wrote:
>>
>> 05.06.2020 18:41, Chris Sosnin пишет:
>>> Currently there is no efficient way to do this.
>>>
>>> Needed for tarantool/tarantool#4415
>> Thanks for the patch!
>>> ---
>>>   decNumber.c | 30 ++++++++++++++++++++++++++++++
>>>   decNumber.h |  1 +
>>>   2 files changed, 31 insertions(+)
>>>
>>> diff --git a/decNumber.c b/decNumber.c
>>> index 7c53bc3..f4ad927 100644
>>> --- a/decNumber.c
>>> +++ b/decNumber.c
>>> @@ -501,6 +501,36 @@ uLong decNumberToUInt64(const decNumber *dn, decContext *set) {
>>>     return 0;
>>>     }  // decNumberToUInt64
>>>   +Flag decNumberIsWhole(const decNumber *dn) {
>>> +  const Unit *up=dn->lsu;
>>> +  if (dn->exponent>=0) {
>>> +    return 1;
>>> +    }
>>> +   else {
>>> +    Int count=-dn->exponent;
>>> +    // spin up whole units until reach the Unit with the unit digit
>>> +    for (; count>=DECDPUN; up++) {
>>> +      if (*up!=0) return 0;
>>> +      count-=DECDPUN;
>>> +      }
>>> +    if (count==0) return 1;             // [a multiple of DECDPUN]
>>> +     else {                             // [not multiple of DECDPUN]
>>> +      Int rem, theInt=0;                          // work
>>> +      // slice off fraction digits and check for non-zero
>>> +      #if DECDPUN<=4
>>> +        theInt=QUOT10(*up, count);
>>> +        rem=*up-theInt*powers[count];
>>> +      #else
>>> +        rem=*up%powers[count];          // slice off discards
>>> +        theInt=*up/powers[count];
>>> +      #endif
>>> +      if (rem!=0) return 0;
>>> +      up++;
>>> +      }
>>> +    }
>>> +    return 1;
>>> +  }  // decNumberIsWhole
>>> +
>> Consider this  diff:
>>
>> diff --git a/decNumber.c b/decNumber.c
>> index f4ad927..acc3fb1 100644
>> --- a/decNumber.c
>> +++ b/decNumber.c
>> @@ -515,17 +515,14 @@ Flag decNumberIsWhole(const decNumber *dn) {
>>         }
>>       if (count==0) return 1;             // [a multiple of DECDPUN]
>>        else {                             // [not multiple of DECDPUN]
>> -      Int rem, theInt=0;                          // work
>> +      Int rem;                          // work
>>         // slice off fraction digits and check for non-zero
>>         #if DECDPUN<=4
>> -        theInt=QUOT10(*up, count);
>> -        rem=*up-theInt*powers[count];
>> +        rem=*up-QUOT10(*up, count)*powers[count];
>>         #else
>>           rem=*up%powers[count];          // slice off discards
>> -        theInt=*up/powers[count];
>>         #endif
>>         if (rem!=0) return 0;
>> -      up++;
>>         }
>>       }
>>       return 1;
> I agree, looks much better, thanks!
> Applied on the branch.

Thanks for the fixes! Both patches LGTM.

>
>>>   /* ------------------------------------------------------------------ */
>>>   /* to-scientific-string -- conversion to numeric string               */
>>>   /* to-engineering-string -- conversion to numeric string              */
>>> diff --git a/decNumber.h b/decNumber.h
>>> index 85a3f3f..71ab27f 100644
>>> --- a/decNumber.h
>>> +++ b/decNumber.h
>>> @@ -169,6 +169,7 @@
>>>     decNumber  * decNumberTrim(decNumber *);
>>>     const char * decNumberVersion(void);
>>>     decNumber  * decNumberZero(decNumber *);
>>> +  uint8_t      decNumberIsWhole(const decNumber *dn);
>>>       /* Functions for testing decNumbers (normality depends on context)  */
>>>     int32_t decNumberIsNormal(const decNumber *, decContext *);
>> -- 
>> Serge Petrenko

-- 
Serge Petrenko

Re: [Tarantool-patches] [PATCH 2/2] Add IsWhole method for checking the fractional part of a number

[Vladislav Shpilevoy]

I guess you rather need name IsInt. 'Whole' has a different
meaning.

Why do you need it, btw? Why decNumberToInt64() and decNumberToUInt64()
are not enough? The only usecase for this new function I can
imagine is an attempt to shrink a decimal value into a normal integer.
Which are either int64_t or uint64_t.

Re: [Tarantool-patches] [PATCH 2/2] Add IsWhole method for checking the fractional part of a number

[Chris Sosnin]

Thank you for the review!

> On 11 Jun 2020, at 20:06, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
> 
> I guess you rather need name IsInt. 'Whole' has a different
> meaning.

You can say “whole number” — a number without decimal part.
But IsInt sounds better, I agree.

> 
> Why do you need it, btw? Why decNumberToInt64() and decNumberToUInt64()
> are not enough? The only usecase for this new function I can
> imagine is an attempt to shrink a decimal value into a normal integer.
> Which are either int64_t or uint64_t.

In case of doubles sqlVdbeIntValue returns 0 if an argument has no decimal part (and doesn’t overflow) and not 0 otherwise, but it always modifies a pointer to an integer, because some functions may still use the result of conversion. So I tried to make the behavior the same.

decimal_to_* functions always trim the value and don’t perform the check cause of decimal_to_integer call, decNumber ones require context (these also don’t really check for zero-decimal part — instead comparing exponent with 0, making 1.000 a bad argument), so we need new wrappers which will do almost the same thing but with an extra check.