Fixing bug on assigning with 0 row matches (#2860)

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
 
 

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  #

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; i<targetlen; i++) {
       if ((INTEGER(rows)[i]<0 && INTEGER(rows)[i]!=NA_INTEGER) || INTEGER(rows)[i]>nrow)
         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);