Skip to content

DT[TRUE] can lead to invalid key #3215

Description

@mattdowle

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 :

DT = data.table(id = 1:5, key="id")
DT1 = DT[TRUE]
key(DT1)
[1] "id"
DT1[3, id:=6L]
key(DT1)
# NULL              # correct
DT$id
# [1] 1 2 6 4 5     # should be 1:5
key(DT)
# [1] "id"          # invalid key

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.

Activity

  1. added this to the 1.12.0 milestone on Dec 13, 2018
  2. renkun-ken commented on Dec 13, 2018

    @renkun-ken
    Member

    Yes, setkey, changing existing columns should not be used on the shallow copy since columns themselves are not copied.

  3. modified the milestones: 1.12.0, 1.12.2 on Jan 6, 2019
  4. jangorecki commented on Feb 4, 2019

    @jangorecki
    Member

    If we won't allow to make shallow copy with dt[TRUE] this issue will be automatically resolved.

  5. mattdowle commented on Feb 26, 2019

    @mattdowle
    MemberAuthor

    Eventually. But in the meantime, we can't break @renkun-ken's workflow.
    More detail here: #3214 (comment)

  6. modified the milestones: 1.12.2, 1.12.4 on Feb 26, 2019
  7. jangorecki commented on Sep 7, 2019

    @jangorecki
    Member

    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)
  8. modified the milestones: 1.12.4, 1.13.0 on Sep 19, 2019
  9. modified the milestones: 1.12.7, 1.12.9 on Dec 8, 2019
  10. modified the milestones: 1.13.1, 1.13.3 on Oct 17, 2020
  11. modified the milestones: 1.14.3, on Jul 19, 2022
  12. modified the milestones: , 1.15.1 on Oct 29, 2023
  13. removed this from the 1.16.0 milestone on Nov 6, 2023
  14. MichaelChirico commented on Jun 15, 2024

    @MichaelChirico
    Member

    Now that we have regex search in GitHub we can get a better sense of how this is used downstream:

    https://github.com/search?q=lang%3AR+%2F%28%5B%5E%21%3D%3E%3C%5D%3D%7C%3C-%29%5B+%5D*%5Ba-z0-9._%5D%2B%5C%5B%5B+%5D*TRUE%5B+%5D*%5C%5D%2F&type=code

    (a bit hard to ensure it's [.data.table statically so this is all "assignment of name[TRUE]")

    Really rare usage pattern indeed. Here's one usage that per the comment assumes [TRUE] is inducing a copy:

    https://github.com/MiDataInt/mdi-apps-framework/blob/dd8124a1d7b00a36a84ae52165df9dc72ecf272b/shiny/shared/session/modules/appControl/historyList/historyList_server.R#L62

    The rest look like other [ methods at a glance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions