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

by sometimes fails when it invokes get #4981

Closed
OfekShilon opened this issue May 8, 2021 · 3 comments · Fixed by #4982
Closed

by sometimes fails when it invokes get #4981

OfekShilon opened this issue May 8, 2021 · 3 comments · Fixed by #4982
Labels
programming parameterizing queries: get, mget, eval, env
Milestone

Comments

@OfekShilon
Copy link
Contributor

> tmp <- data.table(a=c(1,2), b=c(3,4), c=c(1,0))
> tmp[,.(suma=sum(a)), keyby=.(b=get("b"),c)]   # succeeds
   b c suma
1: 3 1    1
2: 4 0    2
> tmp[2,.(suma=sum(a)), keyby=.(b=b,c)]  # succeeds
   b c suma
1: 4 0    2
> tmp[2,.(suma=sum(a)), keyby=.(b=get("b"))]   # succeeds
   b suma
1: 4    2
> tmp[2,.(suma=sum(a)), keyby=.(b=get("b"),c)]   # Fails! - when `i` is non empty and `keyby` contains an additional direct column
Error in get("b") : object 'b' not found

# Output of sessionInfo()
R version 3.4.0 (--) # Reproduces on R4.0.0 too
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 18.04.5 LTS

Matrix products: default
BLAS: /opt/R-3.4.0.mkl/lib64/R/lib/libR.so
LAPACK: /opt/R-3.4.0.mkl/lib64/R/lib/libRlapack.so

locale:
[1] LC_CTYPE=en_US.UTF-8 LC_NUMERIC=C LC_TIME=C LC_COLLATE=C LC_MONETARY=C
[6] LC_MESSAGES=C LC_PAPER=C LC_NAME=C LC_ADDRESS=C LC_TELEPHONE=C
[11] LC_MEASUREMENT=C LC_IDENTIFICATION=C

attached base packages:
[1] stats graphics grDevices utils datasets methods base

other attached packages:
[1] data.table_1.14.1

loaded via a namespace (and not attached):
[1] compiler_3.4.0 tools_3.4.0

OfekShilon added a commit to IstraResearch/data.table that referenced this issue May 8, 2021
@jangorecki jangorecki added the programming parameterizing queries: get, mget, eval, env label May 8, 2021
@jangorecki
Copy link
Member

jangorecki commented May 8, 2021

Thanks for the report. Another example of the a maintenance cost of get for meta programming feature. I hope we can eliminate all such problems by shifting interface towards #4304

@OfekShilon
Copy link
Contributor Author

@jangorecki for some reason my PR wasn't automatically linked to the bug but you can see a simple suggested fix at my branch, mentioned above.

OfekShilon added a commit to IstraResearch/data.table that referenced this issue May 9, 2021
@OfekShilon
Copy link
Contributor Author

Note that the suggested fix checks explicitly for calls to get/eval and some siblings - as do some other places in [.data.table. This is a 'best effort' fix, and won't work if e.g. the by expression calls a function that calls get/eval.
As @jangorecki says, this is probably the best we can do if we keep trying to support parsing get/eval in DT arguments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
programming parameterizing queries: get, mget, eval, env
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants