Repository navigation
DT[TRUE] can lead to invalid key #3215
Copy link
Copy link
Open
Labels
Description
Activity
Yes, setkey, changing existing columns should not be used on the shallow copy since columns themselves are not copied.
Reacted by Xianying Tan, Matt Dowle and Benjamin SchwendingerIf we won't allow to make shallow copy with
dt[TRUE]this issue will be automatically resolved.Reacted by Arun SrinivasanEventually. But in the meantime, we can't break @renkun-ken's workflow.
More detail here: #3214 (comment)the following code could be added to tests to ensure copy behaviour
DT = data.table(a=c(1,2), b=c("b","a")) address(DT) address(DT[]) address(DT[, .SD]) address(DT[TRUE]) sapply(DT, address) sapply(DT[], address) sapply(DT[, .SD], address) sapply(DT[TRUE], address)
Now that we have regex search in GitHub we can get a better sense of how this is used downstream:
(a bit hard to ensure it's
[.data.tablestatically so this is all "assignment ofname[TRUE]")Really rare usage pattern indeed. Here's one usage that per the comment assumes
[TRUE]is inducing a copy:The rest look like other
[methods at a glance.
I didn't realize
DT[TRUE]was a way to achieve a shallow copy. shallow copy was only intended for internal use. Thanks to @renkun-ken for highlighting this in #3214, and related #2254.In v1.11.8 we see this :
It only occurs after
DT[TRUE], iiuc, which hopefully folk have not discovered or relied on too much?! I hope the usage out there is like @renkun-ken described to add new columns to the shallow copy, not to change existing columns!New test 1542.08 was added in PR #2313 ready for when this is fixed.