setDT and [,:=] should share code for re-assigning data.tables

These two regions are close to identical:

data.table/R/data.table.R

Lines 1224 to 1236 in 70c64ac

    
           } else if (name %iscall% c('$', '[[') && is.name(name[[2L]])) { 
        
             k = eval(name[[2L]], parent.frame(), parent.frame()) 
        
             if (is.list(k)) { 
        
               origj = j = if (name[[1L]] == "$") as.character(name[[3L]]) else eval(name[[3L]], parent.frame(), parent.frame()) 
        
               if (is.character(j)) { 
        
                 if (length(j)!=1L) stopf("Cannot assign to an under-allocated recursively indexed list -- L[[i]][,:=] syntax is only valid when i is length 1, but its length is %d", length(j)) 
        
                 j = match(j, names(k)) 
        
                 if (is.na(j)) internal_error("item '%s' not found in names of list", origj) # nocov 
        
               } 
        
               .Call(Csetlistelt,k,as.integer(j), x) 
        
             } else if (is.environment(k) && exists(as.character(name[[3L]]), k)) { 
        
               assign(as.character(name[[3L]]), x, k, inherits=FALSE) 
        
             }

data.table/R/data.table.R

Lines 2970 to 2985 in 70c64ac

    
           } else if (name %iscall% c('$', '[[') && is.name(name[[2L]])) { 
        
             # common case is call from 'lapply()' 
        
             k = eval(name[[2L]], parent.frame(), parent.frame()) 
        
             if (is.list(k)) { 
        
               origj = j = if (name[[1L]] == "$") as.character(name[[3L]]) else eval(name[[3L]], parent.frame(), parent.frame()) 
        
               if (length(j) == 1L) { 
        
                 if (is.character(j)) { 
        
                   j = match(j, names(k)) 
        
                   if (is.na(j)) 
        
                     stopf("Item '%s' not found in names of input list", origj) 
        
                 } 
        
               } 
        
               .Call(Csetlistelt,k,as.integer(j), x) 
        
             } else if (is.environment(k) && exists(as.character(name[[3L]]), k)) { 
        
               assign(as.character(name[[3L]]), x, k, inherits=FALSE) 
        
             }

To keep them in sync, the logic should be extracted to an appropriate helper.

Hi @maintainers,

I’m interested in contributing to this issue. I’ve reviewed the functionality of setDT and [:=] and would like to confirm the following before proceeding:

Could you point me to the primary files where setDT and [:=] handle re-assignments?
Are there specific parts of the logic you’d like to see refactored or reused between these two functions?
Should the shared logic be implemented in R, or would a C utility function be more appropriate?
Additionally, do you have specific test cases or benchmarks you’d like to see included in this enhancement?

Thanks for your guidance!

Best regards,
Nipun

Hi Nipun, if you click through on the code blocks above, it will take you to the place in the source code where you'll find the relevant code for this issue.

Refactor do_j_names to reuse logic from setDT (Rdatatable#6702)

@MichaelChirico

Hi @MichaelChirico ,
I was going through issue #6864 and noticed that solving it would require addressing the problem discussed in PR #6802. Since that PR has been inactive for a while, I’d like to take it over and complete it. After that, I will work on issue #6864 as well.

Before I proceed, should I go ahead with taking over the PR, or would you prefer to wait for the original PR to be completed?

My approach will be to extract the duplicated logic for reassigning data.tables in both setDT and [:=] into a shared helper function. This function will handle cases where the assignment target is a list or environment accessed via $ or [[, ensuring both locations use the same code path and error handling.

I’ll update both sections in data.table.R to call this new helper and ensure all existing tests pass.

Please let me know if you have any suggestions or preferences before I get started. Thanks!

Navigation Menu

Repository navigation

setDT and [,:=] should share code for re-assigning data.tables #6702

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	} else if (name %iscall% c('$', '[[') && is.name(name[[2L]])) {
	k = eval(name[[2L]], parent.frame(), parent.frame())
	if (is.list(k)) {
	origj = j = if (name[[1L]] == "$") as.character(name[[3L]]) else eval(name[[3L]], parent.frame(), parent.frame())
	if (is.character(j)) {
	if (length(j)!=1L) stopf("Cannot assign to an under-allocated recursively indexed list -- L[[i]][,:=] syntax is only valid when i is length 1, but its length is %d", length(j))
	j = match(j, names(k))
	if (is.na(j)) internal_error("item '%s' not found in names of list", origj) # nocov
	}
	.Call(Csetlistelt,k,as.integer(j), x)
	} else if (is.environment(k) && exists(as.character(name[[3L]]), k)) {
	assign(as.character(name[[3L]]), x, k, inherits=FALSE)
	}

	} else if (name %iscall% c('$', '[[') && is.name(name[[2L]])) {
	# common case is call from 'lapply()'
	k = eval(name[[2L]], parent.frame(), parent.frame())
	if (is.list(k)) {
	origj = j = if (name[[1L]] == "$") as.character(name[[3L]]) else eval(name[[3L]], parent.frame(), parent.frame())
	if (length(j) == 1L) {
	if (is.character(j)) {
	j = match(j, names(k))
	if (is.na(j))
	stopf("Item '%s' not found in names of input list", origj)
	}
	}
	.Call(Csetlistelt,k,as.integer(j), x)
	} else if (is.environment(k) && exists(as.character(name[[3L]]), k)) {
	assign(as.character(name[[3L]]), x, k, inherits=FALSE)
	}

setDT and [,:=] should share code for re-assigning data.tables #6702

Description

Activity

nipun-gupta-3108 commented on Jan 21, 2025

MichaelChirico commented on Jan 21, 2025

venom1204 commented on Apr 19, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions