diff --git a/NEWS.md b/NEWS.md index e12a8a54cd..afbc57a619 100644 --- a/NEWS.md +++ b/NEWS.md @@ -5,6 +5,8 @@ #### BUG FIXES +1. Empty RHS of := is no longer an error when the i clause returns no rows to assign to anyway, [#2829](https://github.com/Rdatatable/data.table/issues/2829). Thanks to @cguill95 for reporting and to @MarkusBonsch for fixing. + #### NOTES diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index ce654dde1e..bf03ca0ce8 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -11788,6 +11788,14 @@ test(1909.6, nchar(ans$V2), INT(3287,3287,3287)) DT = data.table(x=1:10, y=1:2) test(1910, DT[, v:=cumsum(x), by="y"], data.table(x=1:10, y=1:2, v=INT(1,2,4,6,9,12,16,20,25,30))) +# testing issue #2829 (assigning to 0 rows) +DT = data.table(COL_INT = 1L, COL_INT_2 = 5L) +test(1911.1, + DT[COL_INT == 0L, c("COL_INT", "NEW_COL"):=list(COL_INT_2, "Test")], + data.table(COL_INT = 1L, COL_INT_2 = 5L, NEW_COL = NA_character_)) +test(1911.2, + DT[, COL_INT := integer(0)], + error = "RHS of assignment to existing column 'COL_INT' is zero length but not NULL.*") ################################### # Add new tests above this line # diff --git a/src/assign.c b/src/assign.c index c919294ef7..1f5d9161a7 100644 --- a/src/assign.c +++ b/src/assign.c @@ -277,7 +277,7 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values, SEXP v // newcolnames : add these columns (if any) // cols : column names or numbers corresponding to the values to set // rows : row numbers to assign - R_len_t i, j, nrow, targetlen, vlen, r, oldncol, oldtncol, coln, protecti=0, newcolnum, indexLength; + R_len_t i, j, nrow, numToDo, targetlen, vlen, r, oldncol, oldtncol, coln, protecti=0, newcolnum, indexLength; SEXP targetcol, RHS, names, nullint, thisvalue, thisv, targetlevels, newcol, s, colnam, class, tmp, colorder, key, index, a, assignedNames, indexNames; SEXP bindingIsLocked = getAttrib(dt, install(".data.table.locked")); Rboolean verbose = LOGICAL(verb)[0], anytodelete=FALSE, isDataTable=FALSE; @@ -316,6 +316,7 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values, SEXP v if (oldncol<1) error("Cannot use := to add columns to a null data.table (no columns), currently. You can use := to add (empty) columns to a 0-row data.table (1 or more empty columns), though."); nrow = length(VECTOR_ELT(dt,0)); if (isNull(rows)) { + numToDo = nrow; targetlen = nrow; if (verbose) Rprintf("Assigning to all %d rows\n", nrow); // fast way to assign to whole column, without creating 1:nrow(x) vector up in R, or here in C @@ -328,7 +329,7 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values, SEXP v if (!isInteger(rows)) error("i is type '%s'. Must be integer, or numeric is coerced with warning. If i is a logical subset, simply wrap with which(), and take the which() outside the loop if possible for efficiency.", type2char(TYPEOF(rows))); targetlen = length(rows); - int numToDo = 0; + numToDo = 0; for (i=0; inrow) error("i[%d] is %d which is out of range [1,nrow=%d].",i+1,INTEGER(rows)[i],nrow); @@ -408,9 +409,9 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values, SEXP v else colnam = STRING_ELT(newcolnames,coln-length(names)); if (coln+1 <= oldncol && isNull(thisvalue)) continue; // delete existing column(s) afterwards, near end of this function if (vlen<1 && nrow>0) { - if (coln+1 <= oldncol) { + if (coln+1 <= oldncol && numToDo > 0) { // numToDo > 0 fixes #2829, see test 1911 error("RHS of assignment to existing column '%s' is zero length but not NULL. If you intend to delete the column use NULL. Otherwise, the RHS must have length > 0; e.g., NA_integer_. If you are trying to change the column type to be an empty list column then, as with all column type changes, provide a full length RHS vector such as vector('list',nrow(DT)); i.e., 'plonk' in the new column.", CHAR(STRING_ELT(names,coln))); - } else if (TYPEOF(thisvalue)!=VECSXP) { // list() is ok for new columns + } else if (coln+1 > oldncol && TYPEOF(thisvalue)!=VECSXP) { // list() is ok for new columns newcolnum = coln-length(names); if (newcolnum<0 || newcolnum>=length(newcolnames)) error("Internal logical error. length(newcolnames)=%d, length(names)=%d, coln=%d", length(newcolnames), length(names), coln);