retain unused levels from zero-row data.table in rbindlist

NEWS.md

@@ -112,6 +112,8 @@
 
 14. Subassigning using `$<-` to a `data.table` embedded in a list column of a single-row `data.table` could fail, [#3474](https://github.com/Rdatatable/data.table/issues/3474). Note that `$<-` is not recommended; please use `:=` instead which already worked in this case. Thanks to Jakob Richter for reporting.
 
+15. `rbind` and `rbindlist` of zero-row items now retain (again) the unused levels of any (zero-length) factor columns, [#3508](https://github.com/Rdatatable/data.table/issues/3508). This was a regression in v1.12.2 just for zero-row items. Unused factor levels were already retained for items having `nrow>=1`. Thanks to Gregory Demin for reporting.
+
 #### NOTES
 
 1. `rbindlist`'s `use.names="check"` now emits its message for automatic column names (`"V[0-9]+"`) too, [#3484](https://github.com/Rdatatable/data.table/pull/3484). See news item 5 of v1.12.2 below.

inst/tests/tests.Rraw

@@ -14847,6 +14847,15 @@ test(2049.2, outer$ab, list(data.table(a=1:3, b=4L)))
 test(2049.3, outer$ab[[1]][, b := 5L], data.table(a=1:3, b=5L))
 test(2049.4, outer$ab, list(data.table(a=1:3, b=5L)))
 
+# rbindlist zero row DT should retain its (unused) levels, #3508
+DT = data.table(f = factor(c("a", "b", "c")))
+test(2050.1, rbind(DT[1], DT[1])[,levels(f)], c("a","b","c"))  # ok before (unused levels when nrow>0 were retained)
+test(2050.2, rbind(DT[1], DT[0])[,levels(f)], c("a","b","c"))  # ok before
+test(2050.3, rbind(DT[0], DT[1])[,levels(f)], c("a","b","c"))  # ok before
+test(2050.4, rbind(DT[0], DT[0])[,levels(f)], c("a","b","c"))  # now ok again (only when nrow=0 were unused levels dropped)
+test(2050.5, rbindlist(list(DT[0], DT[0]))[,levels(f)], c("a","b","c"))  # now ok again
+test(2050.6, rbind(DT[1], data.table(f=factor(letters[10:11]))[0])[,levels(f)], c("a","b","c","j","k")) # now includes "j","k" again
+
 
 ###################################
 #  Add new tests above this line  #

src/rbindlist.c

@@ -392,13 +392,13 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg)
       }
       for (int i=0; i<LENGTH(l); ++i) {
         const int thisnrow = eachMax[i];
-        if (thisnrow==0) continue;
         SEXP li = VECTOR_ELT(l, i);
+        if (!length(li)) continue;  // NULL items in the list() of DT/DF; not if thisnrow==0 because we need to retain (unused) factor levels (#3508)
         int w = usenames ? colMap[i*ncol + j] : j;
-        SEXP thisCol;
-        if (w==-1 || !length(thisCol=VECTOR_ELT(li, w))) {  // !length for zeroCol warning above; #1871
+        if (w==-1) {
           writeNA(target, ansloc, thisnrow);
         } else {
+          SEXP thisCol = VECTOR_ELT(li, w);
           SEXP thisColStr = isFactor(thisCol) ? getAttrib(thisCol, R_LevelsSymbol) : (isString(thisCol) ? thisCol : VECTOR_ELT(coercedForFactor, i));
           const int n = length(thisColStr);
           const SEXP *thisColStrD = STRING_PTR(thisColStr);  // D for data
@@ -464,14 +464,14 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg)
       } else {
         setAttrib(target, R_ClassSymbol, ScalarString(char_factor));
       }
-    } else {
+    } else {  // factor==false
       for (int i=0; i<LENGTH(l); ++i) {
         const int thisnrow = eachMax[i];
         if (thisnrow==0) continue;
         SEXP li = VECTOR_ELT(l, i);
         int w = usenames ? colMap[i*ncol + j] : j;
         SEXP thisCol;
-        if (w==-1 || !length(thisCol=VECTOR_ELT(li, w))) {
+        if (w==-1 || !length(thisCol=VECTOR_ELT(li, w))) {  // !length for zeroCol warning above; #1871
           writeNA(target, ansloc, thisnrow);  // writeNA is integer64 aware and writes INT64_MIN
         } else {
           bool coerced = false;