Skip to content

Commit

Permalink
Closes #703. integer64 support for descending order in DT[order(.)] a…
Browse files Browse the repository at this point in the history
…nd 'setorder()'
  • Loading branch information
arunsrinivasan committed Jun 23, 2014
1 parent 724f1b2 commit f4e9c95
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 3 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ DT[, list(.N, mean(y), sum(y)), by=x] # 1.9.3+ - will use GForce.
* Following `gsum` and `gmean`, now `gmin` and `gmax` from GForce are also implemented. Closes part of #5754 (git [#523](https://github.com/Rdatatable/data.table/issues/523)). Benchmarks are also provided.
i.e., DT[, list(sum(x), min(y), max(z), .N), by=...] # runs by default using GForce

* `setorder()` and `DT[order(.)]` handles `integer64` type in descending order as well. Closes [#703](https://github.com/Rdatatable/data.table/issues/703).

#### BUG FIXES

* When joining to fewer columns than the key has, using one of the later key columns explicitly in j repeated the first value. A problem introduced by v1.9.2 and not caught bythe 1,220 tests, or tests in 37 dependent packages. Test added. Many thanks to Michele Carriero for reporting.
Expand Down
27 changes: 27 additions & 0 deletions inst/tests/tests.Rraw
Original file line number Diff line number Diff line change
Expand Up @@ -4723,6 +4723,33 @@ test(1319.6, DT[order(list(DT$a))], DT[1])
test(1319.7, DT[order(list(DT$a), list(DT$b))], DT[1])
test(1319.8, DT[order(list(DT$a, DT$b))], error="Column '1' is type 'list' which is not")

# FR #703. Not so extensive testing because test 1223 already tests for everything else extensively. Only integer64 here.
# this'll be the test for both DT[order(.)] and setorder(.) as both internally uses forder/forderv
if ("package:bit64" %in% search()) {
set.seed(45L)
DT <- data.table(x=as.integer64(c(-50, 0, 50, 1e18, 1e-18)), y=sample(5))
ans1 <- forder(DT, x, na.last=TRUE, decreasing=FALSE)
ans2 <- forder(DT, x, na.last=FALSE, decreasing=FALSE)
ans3 <- forder(DT, x, na.last=TRUE, decreasing=TRUE)
ans4 <- forder(DT, x, na.last=FALSE, decreasing=TRUE)
test(1320.1, ans1, as.integer(c(1,2,5,3,4)))
test(1320.2, ans2, as.integer(c(1,2,5,3,4)))
test(1320.3, ans3, as.integer(c(4,3,2,5,1)))
test(1320.4, ans4, as.integer(c(4,3,2,5,1)))

set.seed(45L)
DT <- data.table(x=as.integer64(c(-50, 0, NA, 50, 1e18, NA, 1e-18)), y=sample(7))
ans1 <- forder(DT, x, na.last=TRUE, decreasing=FALSE)
ans2 <- forder(DT, x, na.last=FALSE, decreasing=FALSE)
ans3 <- forder(DT, x, na.last=TRUE, decreasing=TRUE)
ans4 <- forder(DT, x, na.last=FALSE, decreasing=TRUE)

test(1320.5, ans1, as.integer(c(1,2,7,4,5,3,6)))
test(1320.6, ans2, as.integer(c(3,6,1,2,7,4,5)))
test(1320.7, ans3, as.integer(c(5,4,2,7,1,3,6)))
test(1320.8, ans4, as.integer(c(3,6,5,4,2,7,1)))
}

##########################

# TO DO: Add test for fixed bug #5519 - dcast.data.table returned error when a package imported data.table, but dint happen when "depends" on data.table. This is fixed (commit 1263 v1.9.3), but not sure how to add test.
Expand Down
15 changes: 12 additions & 3 deletions src/forder.c
Original file line number Diff line number Diff line change
Expand Up @@ -467,15 +467,24 @@ unsigned long long dtwiddle(void *p, int i, int order)
return (nalast == 1 ? ~u.ull : u.ull);
}
unsigned long long mask = (u.ull & 0x8000000000000000) ?
0xffffffffffffffff : 0x8000000000000000; // always flip sign bit and if negative (sign bit was set) flip other bits too
0xffffffffffffffff : 0x8000000000000000; // always flip sign bit and if negative (sign bit was set) flip other bits too
return( (u.ull ^ mask) & dmask2 );
}

unsigned long long i64twiddle(void *p, int i, int order)
// Argument 'order' here is dummy as of now.
// 'order' is in effect now - ascending and descending order implemented. Default
// case (setkey) will not be affected much because order == 1 and nalast != 1
// by default. They're the first two if-statement conditions - shortest checks possible.
// TODO: TO DO: but maybe the condition can be shortened further? too tired to think.
{
u.d = ((double *)p)[i];
return u.ull ^ 0x8000000000000000;
u.ull ^= 0x8000000000000000;
if (order == 1) {// u.ull = 0 now means NA
if (nalast == 1 && u.ull == 0) u.ull ^= 0xFFFFFFFFFFFFFFFF;
} else {
if (!(nalast != 1 && u.ull == 0)) u.ull ^= 0xFFFFFFFFFFFFFFFF;
}
return u.ull;
}

/*
Expand Down

0 comments on commit f4e9c95

Please sign in to comment.