Skip to content

Commit

Permalink
Closes #3723 and #1459 -- use proper NA for integer64 on assign
Browse files Browse the repository at this point in the history
  • Loading branch information
Michael Chirico committed Jul 23, 2019
1 parent 29c270b commit d1c9d89
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 1 deletion.
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,8 @@
24. `column not found` could incorrectly occur in rare non-equi-join cases, [#3635](https:/Rdatatable/data.table/issues/3635). Thanks to @UweBlock for the report.
25. `integer64` defined on a subset of a new column would leave "gibberish" on the remaining rows, [#3723](https:/Rdatatable/data.table/issues/3723). A bug in `rbindlist` with the same root cause was also fixed, [#1459](https:/Rdatatable/data.table/issues/1459). Thanks @shrektan and @jangorecki for the reports.
#### NOTES
1. `rbindlist`'s `use.names="check"` now emits its message for automatic column names (`"V[0-9]+"`) too, [#3484](https:/Rdatatable/data.table/pull/3484). See news item 5 of v1.12.2 below.
Expand Down
8 changes: 8 additions & 0 deletions inst/tests/tests.Rraw
Original file line number Diff line number Diff line change
Expand Up @@ -15428,6 +15428,14 @@ test(2071.10, dcast(data.table(a=1, b=1, l=list(list(1))), a ~ b, value.var='l')
test(2071.11, dcast(data.table(a = 1, b = 2, c = 3), a ~ b, value.var = 'c', fill = '2'),
data.table(a=1, `2`=3, key='a'))

# partial instantiation of integer64 column was creating NA_REAL, not INT64_MIN
if (test_bit64) {
# sub-assign from #3723
test(2072.1, data.table(x=1:2)[1, y := bit64::as.integer64(0L)]$y, as.integer64(c(0L, NA)))
# rbindlist(fill=TRUE) from #1459
test(2072.2, rbind(data.table(a=1:2, b=as.integer64(c(1,NA))), data.table(a=3L), fill=TRUE)$b, as.integer64(c(1, NA, NA)))
}

###################################
# Add new tests above this line #
###################################
Expand Down
10 changes: 9 additions & 1 deletion src/assign.c
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values)
SEXP RHS;

if (coln+1 > oldncol) { // new column
SET_VECTOR_ELT(dt, coln, newcol=allocNAVector(TYPEOF(thisvalue), nrow));
SET_VECTOR_ELT(dt, coln, newcol=allocNAVectorLike(thisvalue, nrow));
// initialize with NAs for when 'rows' is a subset and it doesn't touch
// do not try to save the time to NA fill (contiguous branch free assign anyway) since being
// sure all items will be written to (isNull(rows), length(rows), vlen<1, targetlen) is not worth the risk.
Expand Down Expand Up @@ -1040,6 +1040,14 @@ SEXP allocNAVector(SEXPTYPE type, R_len_t n)
UNPROTECT(1);
return(v);
}
// #3723 -- build NA vector like x -- mainly used to copy attributes pass this to writeNA
SEXP allocNAVectorLike(SEXP x, R_len_t n) {
SEXP v = PROTECT(allocVector(TYPEOF(x), n));
copyMostAttrib(x, v);
writeNA(v, 0, n);
UNPROTECT(1);
return(v);
}

static SEXP *saveds=NULL;
static R_len_t *savedtl=NULL, nalloc=0, nsaved=0;
Expand Down
1 change: 1 addition & 0 deletions src/data.table.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ SEXP SelfRefSymbol;

// assign.c
SEXP allocNAVector(SEXPTYPE type, R_len_t n);
SEXP allocNAVectorLike(SEXP x, R_len_t n);
void writeNA(SEXP v, const int from, const int n);
void savetl_init(), savetl(SEXP s), savetl_end();
int checkOverAlloc(SEXP x);
Expand Down

0 comments on commit d1c9d89

Please sign in to comment.