diff --git a/NEWS.md b/NEWS.md index 8b38cf82cb..83d0c87134 100644 --- a/NEWS.md +++ b/NEWS.md @@ -80,6 +80,8 @@ 18. `example(local=TRUE)` where the example uses `[.data.table` works again (e.g. `example(':=', package='data.table', local=TRUE, echo=FALSE)`), [#7855](https://github.com/Rdatatable/data.table/issues/7855) re-fixing [#2972](https://github.com/Rdatatable/data.table/issues/2972). Thanks @michaelChirico for the fix. +19. `DT[order(double, ..., -non_double, na.last=TRUE)]`, i.e., a double/complex column (in any order) followed by a non-double column in descending order with `na.last=TRUE`, is fixed to respect `na.last` again, [#7875](https://github.com/Rdatatable/data.table/issues/7875). The problematic behavior only occurred under specific conditions on the cardinality of the non-double column. + ### Notes 1. {data.table} now depends on R 3.5.0 (2018). diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 7232c1cd1c..cc801ff6d4 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -21891,3 +21891,24 @@ DF = DF[, 'a', drop=FALSE] test(2382.08, subset(DT, a > 5, select="a", drop=TRUE), subset(DF, a > 5, select="a", drop=TRUE)) test(2382.09, subset(DT, a > 10, select="a", drop=TRUE), subset(DF, a > 10, select="a", drop=TRUE)) test(2382.10, subset(DT, a > 5, drop=TRUE), subset(DF, a > 5, drop=TRUE)) + +# forder() handles na.last=TRUE correctly in ordering double with >1 unique value then non-double (#7875) +DT = data.table( + d0=c(10, 10, 10, 1), + l1=c(FALSE, TRUE, NA, TRUE), + i2=c(2:3, NA, 1L), + d3=c(2, 3, NA, 1), + z4=c(2+1i, 3+1i, NA, 1+1i), + c5=c('b', 'c', NA, 'a') +) +DT_sorted = DT[c(4L, 2:1, 3L)] +test(2383.1, DT[order(d0, -l1, na.last=TRUE)], DT_sorted) +test(2383.2, DT[order(d0, -i2, na.last=TRUE)], DT_sorted) +test(2383.3, DT[order(d0, -d3, na.last=TRUE)], DT_sorted) +test(2383.4, DT[order(d0, -z4, na.last=TRUE)], DT_sorted) +test(2383.5, DT[order(d0, -c5, na.last=TRUE)], DT_sorted) +if (test_bit64) { + DT[, i64 := as.integer64(i2)] + DT_sorted[, i64 := as.integer64(i2)] + test(2383.6, DT[order(d0, -i64, na.last=TRUE)], DT_sorted) +} diff --git a/src/forder.c b/src/forder.c index e7cb7f250c..4ba20f1f0e 100644 --- a/src/forder.c +++ b/src/forder.c @@ -562,7 +562,6 @@ SEXP forder(SEXP DT, SEXP by, SEXP retGrpArg, SEXP retStatsArg, SEXP sortGroupsA STOP(_("Unable to allocate %"PRIu64" bytes of working memory"), (uint64_t)keyAlloc*sizeof(*key)); // # nocov nradix=0; // the current byte we're writing this column to; might be squashing into it (spare>0) int spare=0; // the amount of bits remaining on the right of the current nradix byte - bool isReal=false; bool complexRerun = false; // see comments below in CPLXSXP case SEXP CplxPart = R_NilValue; if (n_cplx) { CplxPart=PROTECT(allocVector(REALSXP, nrow)); n_protect++; } // one alloc is reused for each part @@ -581,6 +580,7 @@ SEXP forder(SEXP DT, SEXP by, SEXP retGrpArg, SEXP retStatsArg, SEXP sortGroupsA } //Rprintf(_("sortType = %d\n"), sortType); hashtab * marks = NULL; // only used for STRSXP below + bool isReal=false; switch(TYPEOF(x)) { case INTSXP : case LGLSXP : // TODO skip LGL and assume range [0,1] range_i32(INTEGER(x), nrow, &min, &max, &na_count);