From 8ade63b7a47cd0e668aa06c2158682409dfd6d54 Mon Sep 17 00:00:00 2001 From: Benjamin Schwendinger Date: Wed, 21 Oct 2026 17:29:37 +0200 Subject: [PATCH] move the cedta fallback outside of [ No behaviour change. --- R/data.table.R | 34 ++++++++++++++++++++-------------- inst/tests/bracket.Rraw | 18 ++++++++++++++++++ 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/R/data.table.R b/R/data.table.R index 10f0b9bc82..658775b231 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -521,6 +521,24 @@ replace_dot_alias = function(e) { list(GForce=GForce, jsub=jsub, jvnames=jvnames) } +# [ called from a scope that is not data.table-aware (!cedta()) behaves like [.data.frame. +# i, j and drop are passed on as promises, so their missingness reaches [.data.frame unchanged. +# nargs is nargs() of the [.data.table call. +.dt_cedta_fallback = function(x, i, j, drop, jsub, nargs, missing_i, missing_drop) { + # Fix for #500 (to do) + if (jsub %iscall% c(":=", "let")) { + # Throw a specific error message + stopf("[ was called on a data.table in an environment that is not data.table-aware (i.e. cedta()), but '%s' was used, implying the owner of this call really intended for data.table methods to be called. See vignette('datatable-importing') for details on properly importing data.table.", as.character(jsub[[1L]])) + } + Nargs = nargs - (!missing_drop) + ans = if (Nargs<3L) { `[.data.frame`(x,i) } # drop ignored anyway by DF[i] + else if (missing_drop) `[.data.frame`(x,i,j) + else `[.data.frame`(x,i,j,drop) + # added is.data.table(ans) check to fix bug #81 + if (!missing_i && is.data.table(ans)) setkey(ans, NULL) # drops index too; tested by plyr::arrange test in other.Rraw + ans +} + "[.data.table" = function(x, i, j, by, keyby, with=TRUE, nomatch=NA, mult="all", roll=FALSE, rollends=if (roll=="nearest") c(TRUE,TRUE) else if (roll>=0.0) c(FALSE,TRUE) else c(TRUE,FALSE), which=FALSE, .SDcols, verbose=getOption("datatable.verbose"), allow.cartesian=getOption("datatable.allow.cartesian"), drop=NULL, on=NULL, env=NULL, showProgress=getOption("datatable.showProgress", interactive())) { # ..selfcount <<- ..selfcount+1 # in dev, we check no self calls, each of which doubles overhead, or could @@ -539,20 +557,8 @@ replace_dot_alias = function(e) { missing_verbose = missing(verbose) # capture parent.frame() changes once as calling scope callenv = parent.frame() - if (!cedta()) { - # Fix for #500 (to do) - if (substitute(j) %iscall% c(":=", "let")) { - # Throw a specific error message - stopf("[ was called on a data.table in an environment that is not data.table-aware (i.e. cedta()), but '%s' was used, implying the owner of this call really intended for data.table methods to be called. See vignette('datatable-importing') for details on properly importing data.table.", as.character(substitute(j)[[1L]])) - } - Nargs = nargs() - (!missing_drop) - ans = if (Nargs<3L) { `[.data.frame`(x,i) } # drop ignored anyway by DF[i] - else if (missing_drop) `[.data.frame`(x,i,j) - else `[.data.frame`(x,i,j,drop) - # added is.data.table(ans) check to fix bug #81 - if (!missing_i && is.data.table(ans)) setkey(ans, NULL) # drops index too; tested by plyr::arrange test in other.Rraw - return(ans) - } + # cedta() and nargs() depend on this frame so are evaluated here + if (!cedta()) return(.dt_cedta_fallback(x, i, j, drop, substitute(j), nargs(), missing_i, missing_drop)) if (!missing_verbose) { if (!is.integer(verbose) && !is.logical(verbose)) stopf("verbose must be logical or integer") if (length(verbose)!=1L || anyNA(verbose)) stopf("verbose must be length 1 non-NA") diff --git a/inst/tests/bracket.Rraw b/inst/tests/bracket.Rraw index 815b231023..398e897fb6 100644 --- a/inst/tests/bracket.Rraw +++ b/inst/tests/bracket.Rraw @@ -171,3 +171,21 @@ test(2.48, X[, lapply(.SD, sum), .SDcols=is.integer], data.table(id=7L, v=10L)) test(2.49, X[, lapply(.SD, sum), .SDcols=!"id"], data.table(t=12, v=10L)) test(2.50, X[, lapply(.SD, sum), .SDcols=-(id:t)], data.table(v=10L)) test(2.51, X[, lapply(.SD, function(x) x/v), .SDcols=!"v"], data.table(id=c(1,0.5,2/3,0.75), t=c(1,2.5,2/3,1))) + +# [ from a scope that is not data.table-aware dispatches to [.data.frame, #852 +DT = data.table(a=1:3, b=c("x","y","z"), key="b") +DF = as.data.frame(DT) +naive = function(expr) eval(substitute(expr), list(DT=DT), new.env(parent=asNamespace("base"))) # use base as namespace to be sure its installed +# results of [.data.frame are not over-allocated, hence compared as data.frame +test(3.01, setDF(naive(DT[2])), DF[2]) +test(3.02, setDF(naive(DT[1:2,])), DF[1:2,]) # slice from front for rownames +test(3.03, setDF(naive(DT[, 2, drop=FALSE])), DF[, 2, drop=FALSE]) +test(3.04, naive(DT[, 2]), DF[, 2]) +test(3.05, naive(DT[2, "a"]), DF[2, "a"]) +test(3.06, key(naive(DT[])), "b") # key retained when i is missing, dropped otherwise +test(3.07, key(naive(DT[1:3, ])), NULL) +test(3.08, naive(DT[, a := 1L]), error="[ was called on a data.table in an environment that is not data.table-aware (i.e. cedta()), but ':=' was used") +test(3.09, naive(DT[, let(a = 1L)]), error="but 'let' was used") +test(3.10, class(naive(DT[2:3, ])), c("data.table","data.frame")) +test(3.11, tryCatch(naive(DT[, a := 1L]), error=conditionCall)[[1L]], quote(`[.data.table`)) # not .dt_cedta_fallback +rm(DT, DF, naive)