Skip to content

Commit

Permalink
gshift works in subset queries (#5963)
Browse files Browse the repository at this point in the history
* add fix for subset

* add NEWS

---------

Co-authored-by: Benjamin Schwendinger <benjamin.schwendinger@tuwien.ac.at>
  • Loading branch information
MichaelChirico and ben-schwen committed Mar 8, 2024
1 parent 815e054 commit e2ce90a
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 2 deletions.
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@

2. `dcast` handles coercion of `fill` to `integer64` correctly, [#4561](https://github.com/Rdatatable/data.table/issues/4561). Thanks to @emallickhossain for the bug report and @MichaelChirico for the fix.

3. Optimized `shift` per group produced wrong results when simultaneously subsetting, for example, `DT[i==1L, shift(x), by=group]`, [#5962](https://github.com/Rdatatable/data.table/issues/5962). Thanks to @renkun-ken for the report and Benjamin Schwendinger for the fix.

## NOTES

1. `transform` method for data.table sped up substantially when creating new columns on large tables. Thanks to @OfekShilon for the report and PR. The implemented solution was proposed by @ColeMiller1.
Expand Down
3 changes: 3 additions & 0 deletions inst/tests/tests.Rraw
Original file line number Diff line number Diff line change
Expand Up @@ -17841,6 +17841,9 @@ test(2224.3, DT[, shift(x), g], error="Type 'list' is not supported by GForce gs
# single vector is un-list()'d, consistently with plain shift(), #5939
DT = data.table(x=1, g=1)
test(2224.4, DT[, .(shift(x)), g], DT[, shift(x), g])
# gshift with subset #5962
DT = data.table(x = 1:3, g = rep(1:2, each=3L))
test(2224.5, DT[g==1L, shift(x), by=g, verbose=TRUE], data.table(g=1L, V1=c(NA,1L,2L)), output="Making each group and running j (GForce TRUE)")

# groupingsets by named by argument
test(2225.1, groupingsets(data.table(iris), j=sum(Sepal.Length), by=c('Sp'='Species'), sets=list('Species')),
Expand Down
4 changes: 2 additions & 2 deletions src/gsumm.c
Original file line number Diff line number Diff line change
Expand Up @@ -1203,7 +1203,7 @@ SEXP gshift(SEXP x, SEXP nArg, SEXP fillArg, SEXP typeArg) {
bool lag;
const bool cycle = stype == CYCLIC;

R_xlen_t nx = xlength(x), nk = length(nArg);
R_xlen_t nk = length(nArg);
if (!isInteger(nArg)) error(_("Internal error: n must be integer")); // # nocov
const int *kd = INTEGER(nArg);
for (int i=0; i<nk; i++) if (kd[i]==NA_INTEGER) error(_("Item %d of n is NA"), i+1);
Expand All @@ -1220,7 +1220,7 @@ SEXP gshift(SEXP x, SEXP nArg, SEXP fillArg, SEXP typeArg) {
}
R_xlen_t ansi = 0;
SEXP tmp;
SET_VECTOR_ELT(ans, g, tmp=allocVector(TYPEOF(x), nx));
SET_VECTOR_ELT(ans, g, tmp=allocVector(TYPEOF(x), n));
#define SHIFT(CTYPE, RTYPE, ASSIGN) { \
const CTYPE *xd = (const CTYPE *)RTYPE(x); \
const CTYPE fill = RTYPE(thisfill)[0]; \
Expand Down

0 comments on commit e2ce90a

Please sign in to comment.