From d8513e79eebd8bfe11cb9c1ee34b1deb8899aef8 Mon Sep 17 00:00:00 2001 From: Benjamin Schwendinger Date: Mon, 22 Dec 2025 23:13:48 +0100 Subject: [PATCH 1/3] add fix to set --- R/data.table.R | 9 +++++++++ inst/tests/tests.Rraw | 3 ++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/R/data.table.R b/R/data.table.R index 3893fe2fc..06c9d738c 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -2848,6 +2848,15 @@ setcolorder = function(x, neworder=key(x), before=NULL, after=NULL, skip_absent= set = function(x,i=NULL,j,value) # low overhead, loopable { + # If removing columns from a table that's not selfrefok, need to call setalloccol first, #7488 + if (is.null(value) || (is.list(value) && any(vapply_1b(value, is.null)))) { + # Removing at least one column + ok = selfrefok(x, verbose=FALSE) + if (ok < 1L) { + # Table is not safe to resize (either fresh from disk or shallow copy) + setalloccol(x, n=eval(getOption("datatable.alloccol")), verbose=FALSE) + } + } .Call(Cassign,x,i,j,NULL,value) invisible(x) } diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 91e6bc580..05844f905 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -21898,7 +21898,8 @@ rm(DT, strings) # do remove columns in freshly unserialized data.tables, #7488 DT = unserialize(serialize(as.data.table(mtcars), NULL)) -test(2351, DT[,carb:=NULL], as.data.table(mtcars)[,carb:=NULL]) +test(2351.1, copy(DT)[,carb:=NULL], as.data.table(mtcars)[,carb:=NULL]) +test(2351.2, set(copy(DT), j="carb", value=NULL), as.data.table(mtcars)[,carb:=NULL]) rm(DT) # rbindlist did not protect the temporary UTF-8 strings, #7452 From 6f8020f805548e36610ec1130781ca1d20a4c83a Mon Sep 17 00:00:00 2001 From: Benjamin Schwendinger Date: Tue, 23 Dec 2025 13:44:30 +0100 Subject: [PATCH 2/3] add assign and tests --- R/data.table.R | 11 +++++------ inst/tests/tests.Rraw | 10 ++++++++-- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/R/data.table.R b/R/data.table.R index 06c9d738c..65bd26f31 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -2849,12 +2849,11 @@ setcolorder = function(x, neworder=key(x), before=NULL, after=NULL, skip_absent= set = function(x,i=NULL,j,value) # low overhead, loopable { # If removing columns from a table that's not selfrefok, need to call setalloccol first, #7488 - if (is.null(value) || (is.list(value) && any(vapply_1b(value, is.null)))) { - # Removing at least one column - ok = selfrefok(x, verbose=FALSE) - if (ok < 1L) { - # Table is not safe to resize (either fresh from disk or shallow copy) - setalloccol(x, n=eval(getOption("datatable.alloccol")), verbose=FALSE) + if ((is.null(value) || (is.list(value) && any(vapply_1b(value, is.null)))) && selfrefok(x, verbose=FALSE) < 1L) { + name = substitute(x) + setalloccol(x, verbose=FALSE) + if (is.name(name)) { + assign(as.character(name), x, parent.frame(), inherits=TRUE) } } .Call(Cassign,x,i,j,NULL,value) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 05844f905..f8e7c5971 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -21898,8 +21898,14 @@ rm(DT, strings) # do remove columns in freshly unserialized data.tables, #7488 DT = unserialize(serialize(as.data.table(mtcars), NULL)) -test(2351.1, copy(DT)[,carb:=NULL], as.data.table(mtcars)[,carb:=NULL]) -test(2351.2, set(copy(DT), j="carb", value=NULL), as.data.table(mtcars)[,carb:=NULL]) +test(2351.1, DT[,carb:=NULL], as.data.table(mtcars)[,carb:=NULL]) +DT = unserialize(serialize(as.data.table(mtcars), NULL)) +test(2351.2, set(DT, j="carb", value=NULL), as.data.table(mtcars)[,carb:=NULL]) +DT = unserialize(serialize(as.data.table(mtcars), NULL)) +null_in_value <- NULL +test(2351.3, "cyl" %notin% names(DT[, cyl := null_in_value]), FALSE) +DT = unserialize(serialize(as.data.table(mtcars), NULL)) +test(2351.4, ncol(DT[, c("cyl", "mpg") := .(null_in_value, null_in_value)]), ncol(mtcars) - 2) rm(DT) # rbindlist did not protect the temporary UTF-8 strings, #7452 From 156f4ff62e77c316bd31b86c048d0794d2848471 Mon Sep 17 00:00:00 2001 From: Benjamin Schwendinger Date: Tue, 23 Dec 2025 13:46:31 +0100 Subject: [PATCH 3/3] update tests --- inst/tests/tests.Rraw | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index f8e7c5971..0eae9856c 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -21903,9 +21903,9 @@ DT = unserialize(serialize(as.data.table(mtcars), NULL)) test(2351.2, set(DT, j="carb", value=NULL), as.data.table(mtcars)[,carb:=NULL]) DT = unserialize(serialize(as.data.table(mtcars), NULL)) null_in_value <- NULL -test(2351.3, "cyl" %notin% names(DT[, cyl := null_in_value]), FALSE) +test(2351.3, "cyl" %notin% names(DT[, cyl := null_in_value])) DT = unserialize(serialize(as.data.table(mtcars), NULL)) -test(2351.4, ncol(DT[, c("cyl", "mpg") := .(null_in_value, null_in_value)]), ncol(mtcars) - 2) +test(2351.4, ncol(DT[, c("cyl", "mpg") := .(null_in_value, null_in_value)]), ncol(mtcars) - 2L) rm(DT) # rbindlist did not protect the temporary UTF-8 strings, #7452