Closes #2046 and closes #2111. Fixes -ve length vectors issue with GF…

…orce (#2480)

NEWS.md

@@ -122,6 +122,7 @@
 
 29. `:=` assignment of one vector to two or more columns, e.g. `DT[, c("x", "y") := 1:10]`, failed to copy the `1:10` data causing errors later if and when those columns were updated by reference, [#2540](https://github.com/Rdatatable/data.table/issues/2540). This is an old issue ([#185](https://github.com/Rdatatable/data.table/issues/185)) that had been fixed but reappeared when code was refactored. Thanks to @patrickhowerter for the detailed report with reproducible example and to @MarkusBonsch for fixing and strengthening tests so it doesn't reappear again.
 
+30. "Negative length vectors not allowed" error when grouping `median` and `var` fixed, [#2046](https://github.com/Rdatatable/data.table/issues/2046) and [#2111](https://github.com/Rdatatable/data.table/issues/2111). Thanks to @caneff and @osofr for reporting and to @kmillar for debugging and explaining the cause.
 
 #### NOTES
 

inst/tests/tests.Rraw

@@ -11252,6 +11252,12 @@ test(1862.1, unique(DT), data.table(A=1:2, B=3:4, key="A"), warning="deprecated
 options(datatable.old.unique.by.key=NULL)
 test(1862.2, unique(DT,by=key(DT)), data.table(A=1:2, B=3:4, key="A"))
 
+# fix for -ve indices issue in gmedian (2046) and gvar (2111)
+DT = data.table(x=c(1,1,1),y=c(3,0,0), key="x")
+test(1863.1, DT[, median(y), by=x], data.table(x=1, V1=0, key="x"))
+DT = data.table(col1 = c(1,1,1, 2,2,2), col2 = c(2,2,2,1,1,1), ID = c(rep(1,3), rep(2,3)), key="ID")
+test(1863.2, DT[, lapply(.SD, var), by=ID], data.table(ID=c(1,2), col1=0, col2=0, key="ID"))
+
 
 ##########################
 

src/gsumm.c

@@ -44,24 +44,25 @@ SEXP gforce(SEXP env, SEXP jsub, SEXP o, SEXP f, SEXP l, SEXP irowsArg) {
   grp = (int *)R_alloc(grpn, sizeof(int));
   // global grp because the g* functions (inside jsub) share this common memory
 
+  maxgrpn = 0;
   if (LENGTH(o)) {
     isunsorted = 1; // for gmedian
     for (g=0; g<ngrp; g++) {
       this = INTEGER(o) + INTEGER(f)[g]-1;
       for (j=0; j<grpsize[g]; j++)  grp[ this[j]-1 ] = g;
+      if (grpsize[g]>maxgrpn) maxgrpn = grpsize[g];  // recalculate (may as well since looping anyway) and check below
     }
   } else {
     for (g=0; g<ngrp; g++) {
       this = grp + INTEGER(f)[g]-1;
       for (j=0; j<grpsize[g]; j++)  this[j] = g;
+      if (grpsize[g]>maxgrpn) maxgrpn = grpsize[g];  // needed for #2046 and #2111 when maxgrpn attribute is not attached to empty o
     }
   }
-  // for gmedian
-  // initialise maxgrpn
-  maxgrpn = INTEGER(getAttrib(o, install("maxgrpn")))[0];
+  SEXP tt = getAttrib(o, install("maxgrpn"));
+  if (length(tt) && INTEGER(tt)[0]!=maxgrpn) error("Internal error: o's maxgrpn mismatches recalculated maxgrpn");
   oo = INTEGER(o);
   ff = INTEGER(f);
-
   irows = INTEGER(irowsArg);
   if (!isNull(irowsArg)) irowslen = length(irowsArg);