Closes #1521, NAs in logical subsets are ignored also in DT[NA].

R/data.table.R

@@ -574,7 +574,6 @@ chmatch2 <- function(x, table, nomatch=NA_integer_) {
                 notjoin = FALSE
                 i = !i
             }
-            if (identical(i,NA)) i = NA_integer_  # see DT[NA] thread re recycling of NA logical
         }
         if (is.null(i)) return( null.data.table() )
         if (is.character(i)) i = data.table(V1=i)   # for user convenience; e.g. DT["foo"] without needing DT[.("foo")]
@@ -689,6 +688,8 @@ chmatch2 <- function(x, table, nomatch=NA_integer_) {
             if (!is.logical(i) && !is.numeric(i)) stop("i has not evaluated to logical, integer or double")
             if (is.logical(i)) {
                 if (isTRUE(i)) irows = i = NULL  # fixes #1249
+		else if (identical(i, NA)) irows=i=integer(0)  # DT[NA] thread recycling of NA logical exists,
+							   # but for #1252 and consistency, we need to return 0-rows
                 else if (length(i)==nrow(x)) irows = i = which(i)   # e.g. DT[colA>3,which=TRUE]
                                                                # also replacing 'i' here - to save memory, #926.
                 else irows=seq_len(nrow(x))[i]  # e.g. recycling DT[c(TRUE,FALSE),which=TRUE], for completeness 

README.md

@@ -144,6 +144,8 @@
 
   41. `by=.EACHI` works as expected along with `mult="first"` and `mult="last"`, [#1287](https://github.com/Rdatatable/data.table/issues/1287) and [#1271](https://github.com/Rdatatable/data.table/issues/1271).
 
+  42. Subsets using logical expressions in `i` never returns all-`NA` rows. Edge case `DT[NA]` is now fixed, [#1252](https://github.com/Rdatatable/data.table/issues/1252). Thanks to @sergiizaskaleta.
+
 #### NOTES
 
   1. Updated error message on invalid joins to reflect the new `on=` syntax, [#1368](https://github.com/Rdatatable/data.table/issues/1368). Thanks @MichaelChirico.

inst/tests/tests.Rraw

@@ -550,9 +550,11 @@ test(205, x[y,list(d),mult="all"][,d], c(1,2,NA,NA))
 # be returned all NA (rarely useful and often confusing, but consistent
 # with data.frame).
 TESTDT = data.table(a=1:3,v=1:3,key="a")
-test(206, TESTDT[NA], data.table(a=NA_integer_,v=NA_integer_,key="a"))  # NA are now allowed in keys, so retains key
+# following #1252, this should return 0-rows. This'll be even more consistent when nomatch=NULL is made default.
+# TODO: this needs to be extended when nomatch=NA etc. is set explicitly
+test(206, TESTDT[NA], data.table(a=integer(0),v=integer(0),key="a"))  # NA are now allowed in keys, so retains key
 setkey(TESTDT,NULL)
-test(207, TESTDT[NA], data.table(a=NA_integer_,v=NA_integer_))
+test(207, TESTDT[NA], data.table(a=integer(0),v=integer(0)))
 
 # With inheritance, NROW and NCOL in base work nicely. No need for them in data.table.
 test(208, NROW(TESTDT), 3L)
@@ -5002,7 +5004,7 @@ test(1344, fread("A,B\n1,T\n2,NA\n3,"), data.table(A=1:3, B=c(TRUE,NA,NA)))
 DT = data.table(a=1:3,b=1:6)
 test(1348, DT[.N], DT[6])
 test(1349, DT[.N-1:3], DT[5:3])
-test(1350, DT[.N+1], DT[NA])
+test(1350, DT[.N+1], DT[NA_integer_]) # following #1252, DT[NA] now returns 0-rows for consistency. This'll also be fixed eventually, when nomatch is made more consistent.
 
 # Adding test to catch any future regressions - #734
 dt = data.table(id = rep(c('a','b'), each=2), val = rep(c(1,2,3), times=c(1,2,1)))