diff --git a/NEWS.md b/NEWS.md index 295b231b80..3c57114fd8 100644 --- a/NEWS.md +++ b/NEWS.md @@ -9,6 +9,8 @@ 2. `keyby=` on columns for which an index exists now uses the index (new feature 7 in v1.11.6 below) but if an `i` subset is present in the same query then it could segfault, [#3062](https://github.com/Rdatatable/data.table/issues/3062). Again thanks to @renkun-ken for reporting. +3. Assigning an out-of-range integer to an item in a factor column (a rare operation) correctly created an `NA` in that spot with warning, but now no longer also corrupts the variable being assigned, [#2984](https://github.com/Rdatatable/data.table/issues/2984). Thanks to @radfordneal for reporting and @MarkusBonsch for fixing. Assigning a string which is missing from the factor levels continues to automatically append the string to the factor levels. + ### Changes in v1.11.6 (on CRAN 19 Sep 2018) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 264b4be07e..497b7ddbed 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -1218,7 +1218,7 @@ DT[1,a:="A"] test(428, DT, data.table(a=factor(c("A","Z","A","Z")),b=1:4)) DT[1,a:=2L] test(429, DT, data.table(a=factor(c("Z","Z","A","Z")),b=1:4)) -test(430, DT[1,1]<- 3L, NA_integer_, warning="RHS contains 3 which is outside the levels range.*1,2.*of column 1, NAs generated") +test(430, DT[1,1]<- 3L, 3L, warning="RHS contains 3 which is outside the levels range.*1,2.*of column 1, NAs generated") test(431, DT[1,1:=4L], data.table(a=factor(c(NA,"Z","A","Z")),b=1:4), warning="RHS contains 4 which is outside the levels range.*1,2.*of column 1, NAs generated") @@ -12239,6 +12239,13 @@ test(1944.4, DT[flag == 1 & group == 1, x], 6L) test(1944.5, indices(DT), "group__flag") test(1944.6, DT[flag == 1, sum(x), keyby = group], data.table(group=1:4, V1=INT(6,3,18,17), key="group")) +## tests for issue #2984 +DT <- data.table(a = factor(c("A", "Z")), b = 1:4) +c <- 3L +DT[1, a:=c] +test(1945.1, c, 3L) +DT[2,1]<- c +test(1945.2, c, 3L) ################################### # Add new tests above this line # diff --git a/src/assign.c b/src/assign.c index c7dadbe243..c1e943d82c 100644 --- a/src/assign.c +++ b/src/assign.c @@ -575,7 +575,11 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values, SEXP v protecti++; // silence warning on singleton NAs if (INTEGER(RHS)[0] != NA_INTEGER) warning("Coerced '%s' RHS to 'integer' to match the factor column's underlying type. Character columns are now recommended (can be in keys), or coerce RHS to integer or character first.", type2char(TYPEOF(thisvalue))); - } else RHS = thisvalue; + } else { + // make sure to copy thisvalue. May be modified below. See #2984 + RHS = PROTECT(duplicate(thisvalue)); + protecti++; + } for (j=0; jLENGTH(targetlevels)) && INTEGER(RHS)[j] != NA_INTEGER) {