Skip to content

Commit

Permalink
by=.EACHI works now when list columns are being returned and some joi…
Browse files Browse the repository at this point in the history
…n values are missing, closes #2300
  • Loading branch information
mattdowle committed Nov 10, 2017
1 parent 0d8d363 commit 862df78
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 1 deletion.
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@

24. Non-equi joins along with `by=.EACHI` returned incorrect result in some rare cases as reported under [#2360](https://github.com/Rdatatable/data.table/issues/2360). This is fixed now. This fix also takes care of [#2275](https://github.com/Rdatatable/data.table/issues/2275). Thanks to @ebs238 for the nice minimal reproducible report, @Mihael for asking on SO and to @Frank for following up on SO and filing an issue.

25. `by=.EACHI` works now when `list` columns are being returned and some join values are missing, [#2300](https://github.com/Rdatatable/data.table/issues/2300). Thanks to @jangorecki and @franknarf1 for the reproducible examples which have been added to the test suite.


#### NOTES

0. The license has been changed from GPL to MPL (Mozilla Public License). All contributors were consulted and approved. [PR#2456](https://github.com/Rdatatable/data.table/pull/2456) details the reasons for the change.
Expand Down
9 changes: 9 additions & 0 deletions inst/tests/tests.Rraw
Original file line number Diff line number Diff line change
Expand Up @@ -11118,6 +11118,15 @@ L = vector("list", 2148)
for (i in seq_along(L)) L[[i]] = DT # many references to the same DT to avoid actually using large RAM for this test
test(1850, rbindlist(L), error="Total rows in the list is 2148000000 which is larger than the maximum number of rows, currently 2147483647")

# by=.EACHI missings to list columns, #2300
dt = data.table(a=factor(1:5, levels=1:10), b=as.list(letters[1:5]))
dt2 = data.table(a=as.factor(1:10))
test(1851.1, dt[dt2, .SD, by=.EACHI, on="a", .SDcols="b"],
data.table(a=as.factor(1:10), b={tmp=vector("list",10); tmp[1:5]=as.list(letters[1:5]); tmp}))
test(1851.2, data.table(a = 1:2, b = list("yo", NULL))[.(1:3), on=.(a), x.b, by = .EACHI],
data.table(a = 1:3, x.b = list("yo", NULL, NULL)))


##########################

# TODO: Tests involving GForce functions needs to be run with optimisation level 1 and 2, so that both functions are tested all the time.
Expand Down
8 changes: 7 additions & 1 deletion src/dogroups.c
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,9 @@ SEXP dogroups(SEXP dt, SEXP dtcols, SEXP groups, SEXP grpcols, SEXP jiscols, SEX
case STRSXP :
SET_STRING_ELT(VECTOR_ELT(SDall,j),0,NA_STRING);
break;
case VECSXP :
SET_VECTOR_ELT(VECTOR_ELT(SDall,j),0,R_NilValue);
break;
default:
error("Logical error. Type of column should have been checked by now");
}
Expand All @@ -177,6 +180,9 @@ SEXP dogroups(SEXP dt, SEXP dtcols, SEXP groups, SEXP grpcols, SEXP jiscols, SEX
case STRSXP :
SET_STRING_ELT(VECTOR_ELT(xSD,j),0,NA_STRING);
break;
case VECSXP :
SET_VECTOR_ELT(VECTOR_ELT(xSD,j),0,R_NilValue);
break;
default:
error("Logical error. Type of column should have been checked by now");
}
Expand Down Expand Up @@ -408,7 +414,7 @@ SEXP dogroups(SEXP dt, SEXP dtcols, SEXP groups, SEXP grpcols, SEXP jiscols, SEX
for (r=0; r<maxn; r++) SET_STRING_ELT(target,thisansloc+r,NA_STRING);
break;
case VECSXP :
for (r=0; r<maxn; r++) SET_VECTOR_ELT(target,thisansloc+r,NA_STRING);
for (r=0; r<maxn; r++) SET_VECTOR_ELT(target,thisansloc+r,R_NilValue);
break;
default:
error("Logical error. Type of column should have been checked by now");
Expand Down

0 comments on commit 862df78

Please sign in to comment.