Skip to content

Commit 741d362

Browse files
j copies list() columns in output (#4890)
* Ensure dt[, col] copies when col is list * Add news item * Add unit test * Fix unit test * Remove now obsolete check for get in jsub * re-use object * errant keystroke * Use = instead of <- * Add test against hidden update-by-reference too * Check is.null() outside the loop first --------- Co-authored-by: Michael Chirico <[email protected]>
1 parent a77e8c2 commit 741d362

File tree

3 files changed

+17
-4
lines changed

3 files changed

+17
-4
lines changed

NEWS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ rowwiseDT(
5959

6060
7. `fread()` performance improves when specifying `Date` among `colClasses`, [#6105](https://github.com/Rdatatable/data.table/issues/6105). One implication of the change is that the column will be an `IDate` (which also inherits from `Date`), which may affect code strongly relying on the column class to be `Date` exactly; computations with `IDate` and `Date` columns should otherwise be the same. If you strongly prefer the `Date` class, run `as.Date()` explicitly following `fread()`. Thanks @scipima for the report and @MichaelChirico for the fix.
6161

62+
8. `dt[, col]` now returns a copy of `col` also when it is a list column, as in any other case, [#4877](https://github.com/Rdatatable/data.table/issues/4877). Thanks to @tlapak for reporting and the PR.
63+
6264
## NOTES
6365

6466
1. Tests run again when some Suggests packages are missing, [#6411](https://github.com/Rdatatable/data.table/issues/6411). Thanks @aadler for the note and @MichaelChirico for the fix.

R/data.table.R

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1382,10 +1382,14 @@ replace_dot_alias = function(e) {
13821382
if (jcpy) jval = copy(jval)
13831383
} else if (address(jval) == address(SDenv$.SD)) {
13841384
jval = copy(jval)
1385-
} else if ( length(jcpy <- which(vapply_1c(jval, address) %chin% vapply_1c(SDenv, address))) ) {
1386-
for (jidx in jcpy) { if(!is.null(jval[[jidx]])) jval[[jidx]] = copy(jval[[jidx]]) }
1387-
} else if (jsub %iscall% 'get') {
1388-
jval = copy(jval) # fix for #1212
1385+
} else {
1386+
sd_addresses = vapply_1c(SDenv, address)
1387+
jcpy = which(!vapply_1b(jval, is.null) & vapply_1c(jval, address) %chin% sd_addresses)
1388+
if (length(jcpy)) {
1389+
for (jidx in jcpy) jval[[jidx]] = copy(jval[[jidx]])
1390+
} else if (address(jval) %chin% sd_addresses) {
1391+
jval = copy(jval) # fix for #4877, includes fix for #1212
1392+
}
13891393
}
13901394
}
13911395

inst/tests/tests.Rraw

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19201,3 +19201,10 @@ test(2286,
1920119201
{ gc(full = TRUE); replicate(10, FALSE); x<4 },
1920219202
`attr<-`(list(3), "class", "foo")),
1920319203
structure(list(1, 2, 3), class = "foo"))
19204+
19205+
# Always return a copy of columns, even when returning a single list column, #4788
19206+
dt = data.table(A=list('a', 'b', 'c'))
19207+
test(2287.1, address(dt[, A]) != address(dt[, A]))
19208+
l = dt[, A]
19209+
dt[1L, A := .(list(1L))]
19210+
test(2287.2, !identical(DT$A[[1L]], l[[1L]]))

0 commit comments

Comments
 (0)