From 9e6236321f44e7b56ff56a431be97d80f9f514f1 Mon Sep 17 00:00:00 2001
From: Benjamin Schwendinger <benjaminschwe@gmail.com>
Date: Sat, 24 Oct 2026 12:43:26 +0200
Subject: [PATCH] move the coercion of i into .coerce_i()

---
 R/data.table.R          | 60 +++++++++++++++++++++++++----------------
 inst/tests/bracket.Rraw | 12 ++++++---
 2 files changed, 46 insertions(+), 26 deletions(-)

diff --git a/R/data.table.R b/R/data.table.R
index 5f95ffea7c..ec9a80d758 100644
--- a/R/data.table.R
+++ b/R/data.table.R
@@ -738,6 +738,42 @@ class(.init_grouped_SDenv) = "dt_query_helper"
 }
 class(.eval_isub) = "dt_query_helper"
 
+# coerce the evaluated i: a one-column numeric matrix becomes integer, a logical i absorbs a not-join, and character, factor,
+#   list and data.frame i become a data.table to join to. Returns .done(null.data.table()) when i is NULL.
+.coerce_i = function(q) {
+  i = q$i
+  notjoin = q$notjoin
+  isnull_inames = q$isnull_inames
+  if (is.matrix(i)) {
+    if (is.numeric(i) && ncol(i)==1L) { # #826 - subset DT on single integer vector stored as matrix
+      i = as.integer(i)
+    } else {
+      stopf("i is invalid type (matrix). Perhaps in future a 2 column matrix could return a list of elements of DT (in the spirit of A[B] in FAQ 2.14). Please report to data.table issue tracker if you'd like this, or add your comments to FR #657.")
+    }
+  }
+  if (is.logical(i)) {
+    if (notjoin) {
+      notjoin = FALSE
+      i = !i
+    }
+  }
+  if (is.null(i)) return(.done(null.data.table()))
+  if (is.character(i) || is.factor(i)) {
+    isnull_inames = TRUE
+    i = data.table(V1=i)   # for user convenience; e.g. DT["foo"] without needing DT[.("foo")]
+  } else if (identical(class(i),"list") && length(i)==1L && is.data.frame(i[[1L]])) { i = as.data.table(i[[1L]]) }
+  else if (identical(class(i),"data.frame")) { i = as.data.table(i) }   # TO DO: avoid these as.data.table() and use a flag instead
+  else if (identical(class(i),"list")) {
+    isnull_inames = is.null(names(i))
+    i = as.data.table(i)
+  }
+  q$i = i
+  q$notjoin = notjoin
+  q$isnull_inames = isnull_inames
+  invisible()
+}
+class(.coerce_i) = "dt_query_helper"
+
 # rows of x covered by the groups: f__/len__ index o__ if x was reordered for grouping, and irows if i subset x
 .group_rows = function(f__, len__, o__, irows) {
   rows = vecseq(f__,len__,NULL)
@@ -1008,29 +1044,7 @@ class(.grouped_make_names) = "dt_query_helper"
     xo = NULL
     isnull_inames = FALSE
     if (.is_done(r <- .eval_isub(q, isub))) return(.done_value(r))
-    if (is.matrix(i)) {
-      if (is.numeric(i) && ncol(i)==1L) { # #826 - subset DT on single integer vector stored as matrix
-        i = as.integer(i)
-      } else {
-        stopf("i is invalid type (matrix). Perhaps in future a 2 column matrix could return a list of elements of DT (in the spirit of A[B] in FAQ 2.14). Please report to data.table issue tracker if you'd like this, or add your comments to FR #657.")
-      }
-    }
-    if (is.logical(i)) {
-      if (notjoin) {
-        notjoin = FALSE
-        i = !i
-      }
-    }
-    if (is.null(i)) return( null.data.table() )
-    if (is.character(i) || is.factor(i)) {
-      isnull_inames = TRUE
-      i = data.table(V1=i)   # for user convenience; e.g. DT["foo"] without needing DT[.("foo")]
-    } else if (identical(class(i),"list") && length(i)==1L && is.data.frame(i[[1L]])) { i = as.data.table(i[[1L]]) }
-    else if (identical(class(i),"data.frame")) { i = as.data.table(i) }   # TO DO: avoid these as.data.table() and use a flag instead
-    else if (identical(class(i),"list")) {
-      isnull_inames = is.null(names(i))
-      i = as.data.table(i)
-    }
+    if (.is_done(r <- .coerce_i(q))) return(.done_value(r))
 
     if (is.data.frame(i)) {
       if (missing_on) {
diff --git a/inst/tests/bracket.Rraw b/inst/tests/bracket.Rraw
index f0ae27e28c..1de293eee9 100644
--- a/inst/tests/bracket.Rraw
+++ b/inst/tests/bracket.Rraw
@@ -191,7 +191,13 @@ test(3.11, tryCatch(naive(DT[, a := 1L]), error=conditionCall)[[1L]], quote(`[.d
 rm(DT, DF, naive)
 
 # evaluation of i moved into .eval_isub(), #852: its errors still report [.data.table
-DT = data.table(x=1:3)
-test(4.1, DT[x == 2L, verbose=TRUE], DT[2L], output="Constructing irows for '!byjoin || nqbyjoin'")
-test(4.2, DT[!(x == 2L), verbose=TRUE], DT[c(1L, 3L)], output="Inverting irows for notjoin")
+# coercion of the evaluated i moved into .coerce_i()
+DT = data.table(a=1:3, b=c("x","y","z"))
+test(4.1, DT[a == 2L, verbose=TRUE], DT[2L], output="Constructing irows for '!byjoin || nqbyjoin'")
+test(4.2, DT[!(a == 2L), verbose=TRUE], DT[c(1L, 3L)], output="Inverting irows for notjoin")
+test(4.3, DT[matrix(1:2, ncol=1)], DT[1:2])
+test(4.4, tryCatch(DT[matrix(1:4, ncol=2L)], error=conditionCall)[[1L]], quote(`[.data.table`))
+test(4.5, DT[!c(TRUE, FALSE, TRUE),], DT[2L])
+test(4.6, DT[list(data.frame(b="y")), on="b",], DT[2L])
+test(4.7, DT[factor("z"), on="b",], DT[3L])
 rm(DT)