From 2bb8d343580f4f0173f2a6ee538df6b4ce96fbab Mon Sep 17 00:00:00 2001 From: Ivan K Date: Sat, 18 Jul 2026 15:20:20 +0300 Subject: [PATCH 1/5] copy(): reimplement in C Don't overwrite list elements unless necessary. Also visit and adjust lists inside attributes. --- R/data.table.R | 22 ++-------------------- src/data.table.h | 2 +- src/init.c | 2 +- src/wrappers.c | 42 +++++++++++++++++++++++++++++++++++++++--- 4 files changed, 43 insertions(+), 25 deletions(-) diff --git a/R/data.table.R b/R/data.table.R index f1eae31ea6..dff01979ab 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -2780,26 +2780,8 @@ sort_by.data.table <- function(x, y, ...) # TO DO, add more warnings e.g. for by.data.table(), telling user what the data.table syntax is but letting them dispatch to data.frame if they want -copy = function(x) { - newx = .Call(Ccopy,x) # copies at length but R's duplicate() also copies truelength over. - # TO DO: inside Ccopy it could reset tl to 0 or length, but no matter as selfrefok detects it - # TO DO: revisit duplicate.c in R 3.0.3 and see where it's at - - reallocate = function(y) { - if (is.data.table(y)) { - .Call(C_unlock, y) - setalloccol(y) - } else if (is.list(y)) { - oldClass = class(y) - setattr(y, 'class', NULL) # otherwise [[.person method (which returns itself) results in infinite recursion, #4620 - y[] = lapply(y, reallocate) - if (!identical(oldClass, 'list')) setattr(y, 'class', oldClass) - } - y - } - - reallocate(newx) -} +copy = function(x) + .Call(Ccopy, x, getOption('datatable.alloccol')) .shallow = function(x, cols = NULL, retain.key = FALSE, unlock = FALSE) { wasnull = is.null(cols) diff --git a/src/data.table.h b/src/data.table.h index dbfe495edb..df46c4a33b 100644 --- a/src/data.table.h +++ b/src/data.table.h @@ -409,7 +409,7 @@ SEXP copyCols(SEXP x, SEXP cols); // where there are no arguments, it must be (void) not () to be a strict prototype SEXP setattrib(SEXP, SEXP, SEXP); SEXP assign(SEXP, SEXP, SEXP, SEXP, SEXP); -SEXP copy(SEXP); +SEXP copy(SEXP, SEXP); SEXP setdt_nrows(SEXP); SEXP alloccolwrapper(SEXP, SEXP, SEXP); SEXP allocrowwrapper(SEXP, SEXP); diff --git a/src/init.c b/src/init.c index 069223b982..3209168c03 100644 --- a/src/init.c +++ b/src/init.c @@ -54,7 +54,7 @@ static const R_CallMethodDef callMethods[] = { {"Cbmerge", (DL_FUNC)&bmerge, -1}, {"Cassign", (DL_FUNC)&assign, -1}, {"Cdogroups", (DL_FUNC)&dogroups, -1}, - {"Ccopy", (DL_FUNC)©, -1}, + {"Ccopy", (DL_FUNC)©, 2}, {"Cshallowwrapper", (DL_FUNC)&shallowwrapper, -1}, {"Csetdt_nrows", (DL_FUNC)&setdt_nrows, -1}, {"Calloccolwrapper", (DL_FUNC)&alloccolwrapper, -1}, diff --git a/src/wrappers.c b/src/wrappers.c index a82bf5f056..2d674919d7 100644 --- a/src/wrappers.c +++ b/src/wrappers.c @@ -57,9 +57,45 @@ SEXP setlevels(SEXP x, SEXP levels, SEXP ulevels) { return(x); } -SEXP copy(SEXP x) -{ - return(duplicate(x)); +struct realloc_nested_dt_attr_ctx { + SEXP x; + int overAlloc; +}; +static SEXP realloc_nested_dt_list(SEXP x, int overAlloc); +static SEXP realloc_nested_dt_list_attr(SEXP tag, SEXP att, void *ctx_) { + struct realloc_nested_dt_attr_ctx *ctx = ctx_; + SEXP newatt = realloc_nested_dt_list(att, ctx->overAlloc); + if (newatt != att) { + PROTECT(newatt); + setAttrib(ctx->x, tag, newatt); + UNPROTECT(1); + } + return R_NilValue; +} +static SEXP realloc_nested_dt_list(SEXP x, int overAlloc) { + int nprot = 0; + if (inherits(x, "data.table")) { + x = PROTECT(alloccol(x, LENGTH(x) + overAlloc, FALSE)); ++nprot; + unlock(x); + } + if (TYPEOF(x) == VECSXP) for (R_xlen_t i = 0; i < XLENGTH(x); ++i) { + SEXP xi = VECTOR_ELT(x, i); + SEXP xinew = realloc_nested_dt_list(xi, overAlloc); + if (xinew != xi) + SET_VECTOR_ELT(x, i, xinew); + } + struct realloc_nested_dt_attr_ctx ctx = { .x = x, .overAlloc = overAlloc }; + R_mapAttrib(x, realloc_nested_dt_list_attr, &ctx); + UNPROTECT(nprot); + return x; +} + +SEXP copy(SEXP x, SEXP overAllocArg) { + int overAlloc = checkOverAlloc(overAllocArg); + PROTECT(x = duplicate(x)); + x = realloc_nested_dt_list(x, overAlloc); + UNPROTECT(1); + return x; } // Internal use only. So that := can update elements of a list of data.table, #2204. Just needed to overallocate/grow the VECSXP. From b22a45677388c07d98325d2dffe2348352620ac9 Mon Sep 17 00:00:00 2001 From: Ivan K Date: Sat, 18 Jul 2026 15:43:31 +0300 Subject: [PATCH 2/5] test 2224.3: replace pairlist columns with VECSXP --- src/wrappers.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/wrappers.c b/src/wrappers.c index 2d674919d7..a5b9ea8413 100644 --- a/src/wrappers.c +++ b/src/wrappers.c @@ -61,10 +61,10 @@ struct realloc_nested_dt_attr_ctx { SEXP x; int overAlloc; }; -static SEXP realloc_nested_dt_list(SEXP x, int overAlloc); +static SEXP realloc_nested_dt_list(SEXP x, int overAlloc, bool); static SEXP realloc_nested_dt_list_attr(SEXP tag, SEXP att, void *ctx_) { struct realloc_nested_dt_attr_ctx *ctx = ctx_; - SEXP newatt = realloc_nested_dt_list(att, ctx->overAlloc); + SEXP newatt = realloc_nested_dt_list(att, ctx->overAlloc, false); if (newatt != att) { PROTECT(newatt); setAttrib(ctx->x, tag, newatt); @@ -72,15 +72,18 @@ static SEXP realloc_nested_dt_list_attr(SEXP tag, SEXP att, void *ctx_) { } return R_NilValue; } -static SEXP realloc_nested_dt_list(SEXP x, int overAlloc) { +static SEXP realloc_nested_dt_list(SEXP x, int overAlloc, bool replacePairlists) { int nprot = 0; if (inherits(x, "data.table")) { x = PROTECT(alloccol(x, LENGTH(x) + overAlloc, FALSE)); ++nprot; unlock(x); } + if (replacePairlists && TYPEOF(x) == LISTSXP) { + x = PROTECT(coerceVector(x, VECSXP)); ++nprot; + } if (TYPEOF(x) == VECSXP) for (R_xlen_t i = 0; i < XLENGTH(x); ++i) { SEXP xi = VECTOR_ELT(x, i); - SEXP xinew = realloc_nested_dt_list(xi, overAlloc); + SEXP xinew = realloc_nested_dt_list(xi, overAlloc, false); if (xinew != xi) SET_VECTOR_ELT(x, i, xinew); } @@ -93,7 +96,7 @@ static SEXP realloc_nested_dt_list(SEXP x, int overAlloc) { SEXP copy(SEXP x, SEXP overAllocArg) { int overAlloc = checkOverAlloc(overAllocArg); PROTECT(x = duplicate(x)); - x = realloc_nested_dt_list(x, overAlloc); + x = realloc_nested_dt_list(x, overAlloc, true); UNPROTECT(1); return x; } From 992c71d8bbe2f6e570c438bfef06dfef7bb050bd Mon Sep 17 00:00:00 2001 From: Ivan K Date: Sat, 18 Jul 2026 18:04:58 +0300 Subject: [PATCH 3/5] NEWS item --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index 025e9a5789..9ebbf60768 100644 --- a/NEWS.md +++ b/NEWS.md @@ -62,6 +62,8 @@ 13. `rbindlist()` (and therefore the `rbind()` method for `data.table`s) no longer raises an error upon encountering more than approximately 50000 columns in a list entry, [#7793](https://github.com/Rdatatable/data.table/issues/7793). The bug was introduced in `data.table` version 1.18.2.1. Thanks to @rickhelmus for the report and @aitap for the fix. +14. `copy()` is now more consistent about reallocating nested `data.table`s, [#7456](https://github.com/Rdatatable/data.table/issues/7456). The resulting list is now only overwritten when necessary, and its attributes are walked in search of data.tables to reallocate as well. Thanks to @be-marc for the report, @david-cortes for additional information, and @aitap for the fix. + ### Notes 1. {data.table} now depends on R 3.5.0 (2018). From e2a4655c9e581a3415b030c61da23a808c6a89b9 Mon Sep 17 00:00:00 2001 From: Ivan K Date: Sat, 18 Jul 2026 19:48:41 +0300 Subject: [PATCH 4/5] Exercise list attributes --- inst/tests/tests.Rraw | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 00973ae3a5..ee1d962be9 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -21831,3 +21831,9 @@ test(2378.92, .Call(CresizeVector, x, 2:3), error = "must be length 1 non-NA non test(2378.93, .Call(CresizeVector, x, -2L), error = "must be length 1 non-NA non-negative integer value") test(2378.94, .Call(CresizeVector, x, NA_integer_), error = "must be length 1 non-NA non-negative integer value") test(2378.95, .Call(CresizeVector, NULL, 2L), error = "must be a vector") + +# copy() reallocates data.tables inside list attributes, #7820 +x = data.table() +y = list() +attr(y, 'dt') = list(x) +test(2379, truelength(attr(copy(y), 'dt')[[1]]) > 0L) From cf6449a4b6e7fe2be7960e52290dace232fc2718 Mon Sep 17 00:00:00 2001 From: Ivan K Date: Sat, 18 Jul 2026 19:59:45 +0300 Subject: [PATCH 5/5] Exercise data.tables as attributes --- inst/tests/tests.Rraw | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index ee1d962be9..e799c50b97 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -21833,7 +21833,7 @@ test(2378.94, .Call(CresizeVector, x, NA_integer_), error = "must be length 1 no test(2378.95, .Call(CresizeVector, NULL, 2L), error = "must be a vector") # copy() reallocates data.tables inside list attributes, #7820 -x = data.table() -y = list() -attr(y, 'dt') = list(x) -test(2379, truelength(attr(copy(y), 'dt')[[1]]) > 0L) +x = list() +attr(x, 'dt') = data.table() +test(2379, truelength(attr(copy(x), 'dt')) > 0L) +rm(x)