diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index dc49a604a5..78a391e108 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -17158,3 +17158,16 @@ for (i in 0:4) test(2155+i/10, data.table(A=1:7, V1=42) ) +# dogroups.c eval(j) could create list columns containing altrep references to the specials, #4759 +# thanks to revdep testing of 1.13.2 where package tstools revealed this via ts() creating ALTREP, #4758 +# the attr(value,"class")<-"newclass" lines mimics a line at the end of stats::ts(). When the +# length(value)>=64, R creates an ALTREP REF wrapper. Which dogroups.c now catches. +# Hence this test needs to be at least 128 rows, 2 groups of 64 each. +DT = data.table(series=c("ts1","ts2"), value=rnorm(128)) +test(2156.1, DT[,list(list({attr(value,"class")<-"newclass";value})),by=series]$V1[[1L]][1L], + DT[1,value]) +test(2156.2, truelength(DT[,list(list(value)),by=series]$V1[[1L]])>=0L) # not -64 carried over by duplicate() of the .SD column +# cover NULL case in copyAsPlain by putting a NULL alongside a dogroups .SD column. The 'if(.GRP==1L)' is just for fun. +test(2156.3, sapply(DT[, list(if (.GRP==1L) list(value,NULL) else list(NULL,value)), by=series]$V1, length), + INT(64,0,0,64)) + diff --git a/src/dogroups.c b/src/dogroups.c index 4c243104a5..6f88c321b6 100644 --- a/src/dogroups.c +++ b/src/dogroups.c @@ -45,7 +45,7 @@ static bool anySpecialStatic(SEXP x) { if (n==0) return false; if (isVectorAtomic(x)) - return TRUELENGTH(x)<0; + return TRUELENGTH(x)<0 || ALTREP(x); if (isNewList(x)) for (int i=0; i make.levels -> rbindlist -> [ -> [.data.table - // Perhaps related to row names and the copyMostAttrib() below is not quite sufficient - - size_t n = XLENGTH(x); - SEXP ans = PROTECT(allocVector(TYPEOF(x), XLENGTH(x))); - switch (TYPEOF(ans)) { + + if (isNull(x)) { + // deal with up front because isNewList(R_NilValue) is true + return R_NilValue; + } + if (!isVectorAtomic(x) && !isNewList(x)) { + // e.g. defer to R the CLOSXP in test 173.3 where a list column item is the function 'mean' + return duplicate(x); + } + const int64_t n = XLENGTH(x); + SEXP ans = PROTECT(allocVector(TYPEOF(x), n)); + switch (TYPEOF(x)) { case RAWSXP: - memcpy(RAW(ans), RAW(x), n*sizeof(Rbyte)); // # nocov; add coverage when ALTREP is turned on for all types - break; // # nocov + memcpy(RAW(ans), RAW(x), n*sizeof(Rbyte)); + break; case LGLSXP: - memcpy(LOGICAL(ans), LOGICAL(x), n*sizeof(Rboolean)); // # nocov - break; // # nocov + memcpy(LOGICAL(ans), LOGICAL(x), n*sizeof(Rboolean)); + break; case INTSXP: memcpy(INTEGER(ans), INTEGER(x), n*sizeof(int)); // covered by 10:1 after test 178 break; @@ -266,22 +267,23 @@ SEXP copyAsPlain(SEXP x) { memcpy(REAL(ans), REAL(x), n*sizeof(double)); // covered by as.Date("2013-01-01")+seq(1,1000,by=10) after test 1075 break; case CPLXSXP: - memcpy(COMPLEX(ans), COMPLEX(x), n*sizeof(Rcomplex)); // # nocov - break; // # nocov + memcpy(COMPLEX(ans), COMPLEX(x), n*sizeof(Rcomplex)); + break; case STRSXP: { const SEXP *xp=STRING_PTR(x); // covered by as.character(as.hexmode(1:500)) after test 642 - for (R_xlen_t i=0; i