detect NAs in isOrderedSubset, closes #3441

Rdatatable · Mar 5, 2019 · b6cb0f5 · b6cb0f5
1 parent 612691a
commit b6cb0f5
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 11 deletions.
diff --git a/R/data.table.R b/R/data.table.R
@@ -1333,13 +1333,7 @@ chmatch2 <- function(x, table, nomatch=NA_integer_) {
           ## check key on i as well!
           ichk = is.data.table(i) && haskey(i) &&
                  identical(head(key(i), length(leftcols)), names(i)[leftcols]) # i has the correct key, #3061
-          if (keylen && (
-            ichk || is.logical(i) || (
-              .Call(CisOrderedSubset, irows, nrow(x)) && !missingroll && ( # !missingroll fix for #3441
-                (roll == FALSE) || length(irows) == 1L
-                )
-              )
-            )) # see #1010. don't set key when i has no key, but irows is ordered and roll != FALSE
+          if (keylen && (ichk || is.logical(i) || (.Call(CisOrderedSubset, irows, nrow(x)) && ((roll == FALSE) || length(irows) == 1L)))) # see #1010. don't set key when i has no key, but irows is ordered and roll != FALSE
             setattr(ans,"sorted",head(key(x),keylen))
         }
         setattr(ans, "class", class(x)) # fix for #5296

diff --git a/src/forder.c b/src/forder.c
@@ -1248,17 +1248,21 @@ SEXP isOrderedSubset(SEXP x, SEXP nrow)
 // specialized for use in [.data.table only
 // Ignores 0s but heeds NAs and any out-of-range (which result in NA)
 {
-  int i=0, last, elem;
   if (!length(x)) return(ScalarLogical(TRUE));
   if (!isInteger(x)) error("x has non-0 length but isn't an integer vector");
   if (!isInteger(nrow) || LENGTH(nrow)!=1 || INTEGER(nrow)[0]<0) error("nrow must be integer vector length 1 and >=0");
   if (LENGTH(x)<=1) return(ScalarLogical(TRUE));
-  while (i<LENGTH(x) && INTEGER(x)[i]==0) i++;
+  int* ix = INTEGER(x);
+  int i;
+  for (i=0; i<LENGTH(x); i++) {
+    if (ix[i]==NA_INTEGER) return(ScalarLogical(FALSE)); // fix for #3441
+    if (ix[i]!=0) break;
+  }
   if (i==LENGTH(x)) return(ScalarLogical(TRUE));
-  last = INTEGER(x)[i];  // the first non-0
+  int last = ix[i];  // the first non-0
   i++;
   for (; i<LENGTH(x); i++) {
-    elem = INTEGER(x)[i];
+    int elem = ix[i];
     if (elem == 0) continue;
     if (elem < last || elem < 0 || elem > INTEGER(nrow)[0])
       return(ScalarLogical(FALSE));