beetween fun edge case regression fix, closes #3565 (#3568)

Rdatatable · May 16, 2019 · d586595 · d586595
1 parent b44e22c
commit d586595
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 10 deletions.
diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw
@@ -14530,6 +14530,10 @@ old = options("datatable.verbose"=TRUE)
 test(2038.18, between(1:5, 2, 4), output="between parallel processing of integer with recycling")
 test(2038.19, between(1:5, 2L, 4), output="between parallel processing of integer with recycling")
 test(2038.20, between(1:5, 2, 4L), output="between parallel processing of integer with recycling")
+# revdep regression #3565
+test(2038.21, between(1:10, -Inf, Inf), between(1:10, NA, NA), output="between parallel processing of double using closed bounds with recycling")
+test(2038.22, between(1:10, -Inf, Inf), between(as.double(1:10), -Inf, Inf), output="between parallel processing of double using closed bounds with recycling")
+
 options(old)
 
 # between int64 support

diff --git a/src/between.c b/src/between.c
@@ -31,16 +31,20 @@ SEXP between(SEXP x, SEXP lower, SEXP upper, SEXP bounds) {
   int nprotect = 0;
   bool integer=true;
   bool integer64=false;
-  if (isInteger(x) &&         // #3517 coerce to num to int when possible
-      (isInteger(lower) || isRealReallyInt(lower)) &&
-      (isInteger(upper) || isRealReallyInt(upper))) {
-    if (!isInteger(lower)) {
-      lower = PROTECT(coerceVector(lower, INTSXP)); nprotect++;
-    }
-    if (!isInteger(upper)) {
-      upper = PROTECT(coerceVector(upper, INTSXP)); nprotect++;
+  if (isInteger(x)) {
+    if ((isInteger(lower) || isRealReallyInt(lower)) &&
+        (isInteger(upper) || isRealReallyInt(upper))) { // #3517 coerce to num to int when possible
+      if (!isInteger(lower)) {
+        lower = PROTECT(coerceVector(lower, INTSXP)); nprotect++;
+      }
+      if (!isInteger(upper)) {
+        upper = PROTECT(coerceVector(upper, INTSXP)); nprotect++;
+      }
+    } else { // #3565
+      x = PROTECT(coerceVector(x, REALSXP)); nprotect++;
     }
-  } else if (inherits(x,"integer64")) {
+  }
+  if (inherits(x,"integer64")) {
     if (!inherits(lower,"integer64") || !inherits(upper,"integer64"))
       error("Internal error in between: 'x' is integer64 while 'lower' and/or 'upper' are not, should have been caught by now"); // # nocov
     integer=false;
@@ -53,7 +57,7 @@ SEXP between(SEXP x, SEXP lower, SEXP upper, SEXP bounds) {
     if (!isReal(upper)) {
       upper = PROTECT(coerceVector(upper, REALSXP)); nprotect++;
     }
-  } else {
+  } else if (!isInteger(x)) {
     error("Internal error in between: 'x' is not int, double or int64, should have been caught by now"); // # nocov
   }
   // TODO: sweep through lower and upper ensuring lower<=upper (inc bounds) and no lower>upper or lower==INT_MAX