Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Any modification on a locked binding should fail, even if operated by reference. #778

Closed
arunsrinivasan opened this issue Aug 21, 2014 · 3 comments
Labels
breaking-change issues whose solution would require breaking existing behavior feature request out-of-scope wontfix

Comments

@arunsrinivasan
Copy link
Member

At the moment, even if a binding is locked, it can still be modified by reference:

ee = new.env();
ee$dt = data.table(x=1:5)
lockBinding("dt", ee)
ee$dt$y <- 6:10 ## error
# Error in ee$dt$y <- 6:10 : cannot change value of locked binding for 'dt'
ee$dt[, y := 6:10] ## works, but shouldn't
#    x  y
#1: 1  6
#2: 2  7
#3: 3  8
#4: 4  9
#5: 5 10

Relevant SO post.

@aryoda
Copy link

aryoda commented Jul 10, 2017

Is there a chance to implement this feature soon?

It is more R'ish (intuitive) than the reference FR #1086 which also proposes a read-only attribute as a possible alternative...

I have gathered all possible references of Stack Overflow questions here: https://stackoverflow.com/questions/44995065/r-how-to-make-a-data-table-read-only-without-copying-it-e-g-for-data-validat

@aryoda
Copy link

aryoda commented Jul 11, 2017

Another unwanted side effect of the current implementation is, that assigning the data.table variable to another new variable (= same memory address) does also modify the "lockBound" first variable:

ee$dt2 <- ee$dt
ee$dt2[, z := 11:15]  # does also modify dt. That shouldn't happen!
ee$dt
# x  y  z
# 1: 1  6 11
# 2: 2  7 12
# 3: 3  8 13
# 4: 4  9 14
# 5: 5 10 15

The implementation of this FR should also prevent this side effect.

@MichaelChirico
Copy link
Member

I'm not sure it's possible to prevent this. I don't see a way to check whether dt is locked in the first example (ee$dt).

We need to know bindingIsLocked("dt", ee), but from within [.data.table, I don't think it's possible to recover, in general, the environment of input x.

e.g.

trace(data.table:::`[.data.table`, at=1, quote(message("x's environment: ", format(environment(x)))))
ee$dt[, y := 6:10]
# Tracing `[.data.table`(ee$dt, , `:=`(y, 6:10)) step 1 
# x's environment: NULL

In this particular case, we can probably back out ee from looking at substitute(x), but it becomes ~impossible very quickly:

foo = function() ee$dt
trace(data.table:::`[.data.table`, at=1, quote(message("substitute(x): ", substitute(x))))
foo()[, y := 6:10]
# Tracing `[.data.table`(foo(), , `:=`(y, 6:10)) step 1 
# substitute(x): foo

Moreover, at the C level, we only have access to R_BindingIsLocked():

https://github.com/r-devel/r-svn/blob/9b23165e6e0b53f01aa22e5f0cf40ac601e10aa4/src/main/envir.c#L3454

Which again requires a SYMSXP and the environment in which it's bound (AFAICT there's no way to pull out the correct environment from C either).

It looks like the BINDING_IS_LOCKED() macro might work, but it's not part of the public API:

https://github.com/r-devel/r-svn/blob/9b23165e6e0b53f01aa22e5f0cf40ac601e10aa4/src/include/Defn.h#L1205

Lastly, I suspect this will wind up breaking a lot of packages, e.g. if you define a data.table in your package's namespace as some sort of cache, and package functions edit it with :=, that will now be broken. I think this is likely to be common in practice.

Therefore, I am closing this as wontfix. If anyone can figure out a way to determine from within [.data.table that an object is locked, please feel free to open a PR, but it will almost surely need to go through a long deprecation cycle before fully stopping locked objects from being edited by data.table.

@MichaelChirico MichaelChirico added wontfix breaking-change issues whose solution would require breaking existing behavior out-of-scope labels Apr 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change issues whose solution would require breaking existing behavior feature request out-of-scope wontfix
Projects
None yet
Development

No branches or pull requests

3 participants