Skip to content

Commit

Permalink
Closes #1926 -- unlock shallow copy of .SD when needed in joins in j
Browse files Browse the repository at this point in the history
  • Loading branch information
Michael Chirico committed Aug 18, 2019
1 parent a8e926a commit fb9cab6
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 0 deletions.
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,8 @@
27. `integer64` sum-by-group is now properly optimized, [#1647](https://github.com/Rdatatable/data.table/issues/1647), [#3464](https://github.com/Rdatatable/data.table/issues/3464). Thanks to @mlandry22-h2o for the report.
28. Joining to `.SD` in `j` was erroring because `bmerge` uses `set` in some cases (e.g. numeric-to-integer coercion or factor joins-on-levels), but the shallow copy of `.SD` was still locked, [#1926](https://github.com/Rdatatable/data.table/issues/1926).
#### NOTES
1. `rbindlist`'s `use.names="check"` now emits its message for automatic column names (`"V[0-9]+"`) too, [#3484](https://github.com/Rdatatable/data.table/pull/3484). See news item 5 of v1.12.2 below.
Expand Down
8 changes: 8 additions & 0 deletions R/bmerge.R
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,15 @@ bmerge = function(i, x, icols, xcols, roll, rollends, nomatch, mult, ops, verbos
# Otherwise, the type of the i column is always returned.

i = shallow(i)
# #1926 -- merge on .SD in i fails _sometimes_ because of set() being done here
dt_lock = '.data.table.locked'
if (isTRUE(attr(i, dt_lock, exact=TRUE))) setattr(i, dt_lock, NULL)
x = shallow(x)
if (isTRUE(attr(x, dt_lock, exact=TRUE))) setattr(x, dt_lock, NULL)
if (isTRUE(attr(callersi, dt_lock, exact=TRUE))) {
setattr(callersi, dt_lock, NULL)
on.exit(setattr(callersi, dt_lock, TRUE))
}
# careful to only plonk syntax (full column) on i/x from now on otherwise user's i and x would change;
# this is why shallow() is very importantly internal only, currently.

Expand Down
9 changes: 9 additions & 0 deletions inst/tests/tests.Rraw
Original file line number Diff line number Diff line change
Expand Up @@ -15671,6 +15671,15 @@ if (test_bit64) {
test(2077.06, int64_int32_match(d[, sum(i32, na.rm=TRUE), g], d[, sum(i64, na.rm=TRUE), g]))
}

# #1926 -- unlock .SD for bmerge in j
DT = data.table(id=1:2, v=3:4)
DT2 = data.table(id=1, x=5)
## DT2.i is numeric so bmerge does coercion with set()
test(2078.1, DT[id == 1, DT2[.SD, on="id"]], data.table(id=1L, x=5, v=3L))
df1 = data.table(a=1:5, b=c(0, 0, 1, 0, 2))
df2 = data.table(c=c(1, 1, 2, 2, 3), d=c(3, 4, 3, 5, 4))
test(2078.2, copy(df2)[ , s := df1[.SD, on=.(a >= c, a <= d), sum(b), by=.EACHI]$V1],
df2[ , s := c(1, 1, 1, 3, 1)])

###################################
# Add new tests above this line #
Expand Down

0 comments on commit fb9cab6

Please sign in to comment.