Fix for the issue introduced while fixing #1477.

Rdatatable · Feb 17, 2016 · b901d1a · b901d1a
1 parent c7a0831
commit b901d1a
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 4 deletions.
diff --git a/R/data.table.R b/R/data.table.R
@@ -1277,7 +1277,7 @@ chmatch2 <- function(x, table, nomatch=NA_integer_) {
                 lenjval = vapply(jval, length, 0L)
                 if (any(lenjval != lenjval[1L])) {
                     jval = as.data.table.list(jval)   # does the vector expansion to create equal length vectors
-                    jvnames = names(jval) # fix for #1477
+                    jvnames = jvnames[lenjval != 0L]  # fix for #1477
                 } else setDT(jval)
             }
             if (is.null(jvnames)) jvnames = character(length(jval)-length(bynames))

diff --git a/README.md b/README.md
@@ -109,7 +109,7 @@
 
   26. Auto indexing returns order of subset properly when input `data.table` is already sorted, [#1495](https://github.com/Rdatatable/data.table/issues/1495). Thanks @huashan for the nice reproducible example.
 
-  27. `[.data.table` handles column subsets based on conditions that result in `NULL` as list elements correctly, [#1477](https://github.com/Rdatatable/data.table/issues/1477). Thanks @MichaelChirico.
+  27. `[.data.table` handles column subsets based on conditions that result in `NULL` as list elements correctly, [#1477](https://github.com/Rdatatable/data.table/issues/1477). Thanks @MichaelChirico. Also thanks to @Max from DSR for spotting a bug as a result of this fix. Now fixed.
 
 #### NOTES
 

diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw
@@ -7389,9 +7389,17 @@ test(1599.1, data.table(x=vv, y=1:10, stringsAsFactors=TRUE)$x, factor(vv))
 vv = sample(c(letters[1:3], NA), 10, TRUE)
 test(1599.2, data.table(x=vv, y=1:10, stringsAsFactors=TRUE)$x, factor(vv))
 
-# bug #1477
+# bug #1477 fix
 DT <- data.table(a = 0L:1L, b = c(1L, 1L))
-test(1600, DT[ , lapply(.SD, function(x) if (all(x)) x)], data.table(b=c(1L, 1L)))
+test(1600.1, DT[ , lapply(.SD, function(x) if (all(x)) x)], data.table(b=c(1L, 1L)))
+# this fix wasn't entirely nice as it introduced another issue.
+# it's fixed now, but adding a test for that issue as well to catch it early next time.
+set.seed(17022016L)
+DT1 = data.table(id1 = c("c", "a", "b", "b", "b", "c"), 
+                  z1 = sample(100L, 6L), 
+                  z2 = sample(letters, 6L))
+DT2 = data.table(id1=c("c", "w", "b"), val=50:52)
+test(1600.2, names(DT1[DT2, .(id1=id1, val=val, bla=sum(z1, na.rm=TRUE)), on="id1"]), c("id1", "val", "bla"))
 
 ##########################