delete rows by reference #7536

ben-schwen · 2025-12-27T23:14:39Z

Closes #635

Currently only supports deletion of i, but could also be integrated into dogroups.

Do we need/want a functional form like delete(x, irows). Depending on what we allow for irows this would need a rewrite/duplicate of the internals of [i evaluation.

codecov · 2025-12-27T23:30:15Z

Codecov Report

❌ Patch coverage is 91.72932% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.91%. Comparing base (dc69b3d) to head (57a4bdc).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
src/deleterows.c	90.09%	10 Missing ⚠️
src/wrappers.c	80.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7536      +/-   ##
==========================================
- Coverage   98.97%   98.91%   -0.06%     
==========================================
  Files          87       88       +1     
  Lines       16733    16853     +120     
==========================================
+ Hits        16561    16670     +109     
- Misses        172      183      +11

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions · 2025-12-27T23:35:46Z

HEAD=delete_by_ref slower P<0.001 for DT[by,verbose=TRUE] improved in #6296

Generated via commit 57a4bdc

Download link for the artifact containing the test results: ↓ atime-results.zip

Task	Duration
R setup and installing dependencies	2 minutes and 41 seconds
Installing different package versions	42 seconds
Running and plotting the test cases	3 minutes and 52 seconds

src/deleterows.c

ben-schwen · 2025-12-28T14:12:50Z

What if columns are not marked as resizable? AFAIU delete rows will have to create a resizable copy in order to remove it. Right? In such case I think we may want to provide a function to mark resizable explicitly, ahead of time, so further processing is more predicable.

Yes. I have added allocrow as a pendant to alloccol. In the future we can also use this to overallocate the number of rows so we go implement the fast insert #660

jangorecki

setallocrow needs to return invisibly.
I was expecting following example to show difference, it does not

x1 = data.table(a = 1:2)
x2 = x1

y1 = data.table(a = 1:2)
invisible(setallocrow(y1))
y2 = y1

delete_first_row = function(x) x[1L, .ROW := NULL]

delete_first_row(x1)
delete_first_row(y1)

x2
y2

could verbose=T tell when object is re-allocated (as resizable) rather than truly updated in place?

we need example that shows why setallocrow() might be desired.

does it make sense to mark resizable already in as.data.table / fread / rbindlist?

ben-schwen · 2025-12-29T12:54:16Z

I have added verbose messages to allocrow. I'm also not convinced how useful allocrow is right now. In the future yes, it will be essential when overallocating the number rows, respectively shortening overallocated rows.

library(data.table)
dt = data.table(a = 1:5)
truelength(dt$a)
#> [1] 0
setallocrow(dt)
truelength(dt$a)
#> [1] 5
dt[3:5, .ROW := NULL]
truelength(dt$a)
#> [1] 5
setallocrow(dt)
truelength(dt$a)
#> [1] 2

^{Created on 2025-12-29 with reprex v2.1.1}

Thinking about the bigger picture of fast insert we will need to think about how much we want to overallocate when inserting respectively when we want to tidy up.

MichaelChirico · 2025-12-29T17:34:59Z

man/assign.Rd


 Note that for efficiency no check is performed for duplicate assignments, i.e. if multiple values are passed for assignment to the same index, assignment to this index will occur repeatedly and sequentially; for a given use case, consider whether it makes sense to create your own test for duplicates, e.g. in production code.

+Note that \code{.ROW := NULL} is a special case used to delete rows by reference. Unlike column assignment, this requires an \code{i} expression to specify which rows to delete, and does not support \code{by} or \code{keyby}. See \code{\link{.ROW}} or \code{\link{special-symbols}} for details.


aside for future consideration -- another use case would be DT[<optional i>, by=<grp>, having=<condition>, .ROW := NULL]

~~also, be sure to point to ?setallocrow as the functional equivalent~~

Oh, I misunderstood

MichaelChirico · 2025-12-29T17:37:44Z

man/special-symbols.Rd

 \description{
    \code{.SD}, \code{.BY}, \code{.N}, \code{.I}, \code{.GRP}, and \code{.NGRP} are \emph{read-only} symbols for use in \code{j}. \code{.N} can be used in \code{i} as well. \code{.I} can be used in \code{by} as well. See the vignettes, Details and Examples here and in \code{\link{data.table}}.
    \code{.EACHI} is a symbol passed to \code{by}; i.e. \code{by=.EACHI}, \code{.NATURAL} is a symbol passed to \code{on}; i.e. \code{on=.NATURAL}
+    \code{.ROW} is a symbol used with \code{:= NULL} to delete rows by reference; i.e. \code{DT[i, .ROW := NULL]} deletes the rows selected by \code{i}.


maybe just point to ?assign, the info here is a bit repetitive

MichaelChirico · 2025-12-29T17:43:39Z

vignettes/datatable-importing.Rmd

We should probably add a section to the Reference semantics vignette about this.

E.g., illustrating any issues that might obtain from shared references to data.tables, what happens here?

DT1 = data.table(i = 1:10) DT2 = DT1 DT2[i > 5, .ROW := NULL] nrow(DT1)

MichaelChirico · 2025-12-29T17:45:28Z

R/data.table.R

+          if (is.null(jsub) || identical(jsub, quote(NULL))) {
+            if (is.null(irows))
+              stopf(".ROW := NULL requires i= condition to specify rows to delete")
+            if (!missingby)
+              stopf(".ROW := NULL with 'by' or 'keyby' is not supported yet")
+            .Call(CdeleteRows, x, irows)
+            return(suppPrint(x))
+          } else {
+            stopf(".ROW can only be used with := NULL to delete rows")
+          }


Suggest unnesting:

Suggested change

if (is.null(jsub) || identical(jsub, quote(NULL))) {

if (is.null(irows))

stopf(".ROW := NULL requires i= condition to specify rows to delete")

if (!missingby)

stopf(".ROW := NULL with 'by' or 'keyby' is not supported yet")

.Call(CdeleteRows, x, irows)

return(suppPrint(x))

} else {

stopf(".ROW can only be used with := NULL to delete rows")

}

if (!is.null(jsub) && !identical(jsub, quote(NULL))) {

stopf(".ROW can only be used with := NULL to delete rows")

if (is.null(irows))

stopf(".ROW := NULL requires i= condition to specify rows to delete")

if (!missingby)

stopf(".ROW := NULL with 'by' or 'keyby' is not supported yet")

.Call(CdeleteRows, x, irows)

return(suppPrint(x))

MichaelChirico · 2025-12-29T17:47:51Z

inst/tests/tests.Rraw

+# multirow
+dt = data.table(a=1:5, b=letters[1:5])
+test(2356.09, copy(dt)[c(1L, 3L), .ROW := NULL], dt[c(2,4,5)])
+test(2356.10, copy(dt)[c(TRUE, FALSE, TRUE, FALSE, TRUE), .ROW := NULL], dt[c(2,4)])


this looks the same as 2356.03?

MichaelChirico · 2025-12-29T17:50:20Z

man/truelength.Rd


    Please note: over-allocation of the column pointer vector is not for efficiency \emph{per se}; it is so that \code{:=} can add columns by reference without a shallow copy.
+
+    \code{setallocrow} is a utility function that prepares columns for fast row operations (delete or insert (not implemented yet)) by reference and manages row capacity.


~~mention the .ROW:=NULL alternative here by linking to ?assign~~ I misunderstood

MichaelChirico · 2025-12-29T17:50:39Z

man/truelength.Rd


    Please note: over-allocation of the column pointer vector is not for efficiency \emph{per se}; it is so that \code{:=} can add columns by reference without a shallow copy.
+
+    \code{setallocrow} is a utility function that prepares columns for fast row operations (delete or insert (not implemented yet)) by reference and manages row capacity.


current wording doesn't make clear that delete is implemented, insert is not.

Suggested change

\code{setallocrow} is a utility function that prepares columns for fast row operations (delete or insert (not implemented yet)) by reference and manages row capacity.

\code{setallocrow} is a utility function that prepares columns for fast row operations (delete or insert) by reference and manages row capacity. (Note that 'insert' by reference is not yet implemented)

MichaelChirico · 2025-12-29T17:57:14Z

man/truelength.Rd

+
+    \code{setallocrow} is a utility function that prepares columns for fast row operations (delete or insert (not implemented yet)) by reference and manages row capacity.
+    Before deleting or inserting rows by reference, columns must be resizable.
+    \code{setallocrow} ensures all columns are in the appropriate state by converting ALTREP columns to materialized form and reallocating


I think this needs a bit of clarification -- when I read it, my thought was "oh, we can't do .ROW:=NULL on a table with ALTREP columns unless we first setallocrow()", but I don't see any examples/tests to that effect.

OTOH, I think (?) we strive to expand all ALTREP columns, so I haven't been able to come up with such an example

MichaelChirico · 2025-12-29T17:59:15Z

src/wrappers.c


+SEXP allocrowwrapper(SEXP dt, SEXP n) {
+  if (!isInteger(n) || length(n)!=1 || INTEGER(n)[0]<0 || INTEGER(n)[0]==NA_INTEGER)
+    error("n must be a single non-negative non-NA integer");


Suggested change

error("n must be a single non-negative non-NA integer");

error(_("n must be a single non-negative non-NA integer"));

MichaelChirico · 2025-12-29T17:59:49Z

src/deleterows.c

+  for (R_xlen_t i = 0; i < length(dt); i++) {
+    SEXP col = VECTOR_ELT(dt, i);
+    if (!isVector(col))
+      error("Cannot make non-vector column %lld resizable", (long long)(i + 1));


Be sure to mark strings for translation as appropriate

Suggested change

error("Cannot make non-vector column %lld resizable", (long long)(i + 1));

error(_("Cannot make non-vector column %lld resizable"), (long long)(i + 1));

MichaelChirico

It looks like this is kind of 2 PRs -- one about setallocrow() and .ROW := NULL, are they totally independent?

Or perhaps, we need setallocrow() first in order to enable .ROW := NULL?

It might be easier to review in that sequence -- PR 1 adds setallocrow() (with no way to actually use it), PR 2 enables DELETE operations by reference. WDYT?

jangorecki · 2025-12-29T18:45:48Z

table that is already marked resizable, when it will get new column added, it should lose/reset it's resizable marker, or make newly added columns resizable:

d = data.table(a = 4:1)
.Internal(inspect(d[["a"]]))
#@55b90c4c1c28 13 INTSXP g0c2 [REF(2)] (len=4, tl=0) 4,3,2,1
setallocrow(d)
.Internal(inspect(d[["a"]]))
#@55b90c4c1e68 13 INTSXP g0c2 [REF(1),gp=0x20] (len=4, tl=4) 4,3,2,1
d[, "x" := c(1,2,3,4)]
.Internal(inspect(d[["a"]]))
#@55b90c4c1e68 13 INTSXP g0c2 [REF(1),gp=0x20] (len=4, tl=4) 4,3,2,1
.Internal(inspect(d[["x"]]))
#@55b910172c58 14 REALSXP g0c3 [REF(2)] (len=4, tl=0) 1,2,3,4

Thinking about the bigger picture of fast insert we will need to think about how much we want to overallocate when inserting respectively when we want to tidy up.

I think it might be explicitly requested from user each time

I was expecting we can show why one might want to use setallocrow, any example? the one I made in my last post appeared to not be working...

ben-schwen added 2 commits December 28, 2025 00:13

add remove rows by reference

9a278cd

remove slip

6d2182a

ben-schwen requested review from HughParsonage, MichaelChirico and tdhock as code owners December 27, 2025 23:14

ben-schwen added 3 commits December 28, 2025 00:15

remove seq comments

b9e76eb

add new line

703456a

fix 64 bit test

6f4867c

ben-schwen added 2 commits December 28, 2025 01:30

add coverage and remove dead code

67d6505

use R_xlen_t consistently

4888272

This comment was marked as resolved.

Sign in to view

jangorecki reviewed Dec 28, 2025

View reviewed changes

src/deleterows.c Outdated Show resolved Hide resolved

ben-schwen added 2 commits December 28, 2025 14:40

remove guard since throttle will set number of threads

8d5e869

add allocrow function

4936003

add documentation for allocrow

e026e8d

jangorecki reviewed Dec 29, 2025

View reviewed changes

add documentation and verbose messages

1309d52

add row overallocation feature

57a4bdc

MichaelChirico reviewed Dec 29, 2025

View reviewed changes


		Note that for efficiency no check is performed for duplicate assignments, i.e. if multiple values are passed for assignment to the same index, assignment to this index will occur repeatedly and sequentially; for a given use case, consider whether it makes sense to create your own test for duplicates, e.g. in production code.

		Note that \code{.ROW := NULL} is a special case used to delete rows by reference. Unlike column assignment, this requires an \code{i} expression to specify which rows to delete, and does not support \code{by} or \code{keyby}. See \code{\link{.ROW}} or \code{\link{special-symbols}} for details.


		Please note: over-allocation of the column pointer vector is not for efficiency \emph{per se}; it is so that \code{:=} can add columns by reference without a shallow copy.

		\code{setallocrow} is a utility function that prepares columns for fast row operations (delete or insert (not implemented yet)) by reference and manages row capacity.

	error("n must be a single non-negative non-NA integer");
	error(_("n must be a single non-negative non-NA integer"));

	error("Cannot make non-vector column %lld resizable", (long long)(i + 1));
	error(_("Cannot make non-vector column %lld resizable"), (long long)(i + 1));

delete rows by reference #7536

Are you sure you want to change the base?

delete rows by reference #7536

Uh oh!

Conversation

ben-schwen commented Dec 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Dec 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Dec 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

ben-schwen commented Dec 28, 2025

Uh oh!

jangorecki left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ben-schwen commented Dec 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MichaelChirico Dec 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MichaelChirico Dec 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MichaelChirico Dec 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MichaelChirico left a comment

Choose a reason for hiding this comment

Uh oh!

jangorecki commented Dec 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ben-schwen commented Dec 27, 2025 •

edited

Loading

codecov bot commented Dec 27, 2025 •

edited

Loading

github-actions bot commented Dec 27, 2025 •

edited

Loading

jangorecki left a comment •

edited

Loading

MichaelChirico Dec 29, 2025 •

edited

Loading

MichaelChirico Dec 29, 2025 •

edited

Loading

MichaelChirico Dec 29, 2025 •

edited

Loading

jangorecki commented Dec 29, 2025 •

edited

Loading