From f47aa3c35645ae0999a16e4b46e0b764c69620c3 Mon Sep 17 00:00:00 2001
From: nitish jha <nitishjha@nitishs-MacBook-Air.local>
Date: Fri, 2 Aug 2024 02:10:28 +0530
Subject: [PATCH 1/8] improved error message in assign

---
 NEWS.md               | 2 ++
 inst/tests/tests.Rraw | 8 ++++----
 src/assign.c          | 2 +-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index fdc0db931b..75fd8078a9 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -186,6 +186,8 @@ This feature resolves [#4387](https://github.com/Rdatatable/data.table/issues/43
 
 24. The internal `init()` function in `fread.c` module has been marked as `static`, [#6328](https://github.com/Rdatatable/data.table/pull/6328). This is to avoid name collisions, and the resulting segfaults, with other libraries that might expose the same symbol name, and be already loaded by the R process. This was observed in Cray HPE environments where the `libsci` library providing LAPACK to R already has an `init` symbol. Thanks to @rtobar for the report and fix.
 
+25. `set()` and `:=` now provide an improved error message when attempting to directly assign `NULL` to a list column item. The new message guides users to use `list(list(NULL))` instead, enhancing clarity and preventing common mistakes. Previously, this operation would throw a generic error, but now the message is more informative and helpful. Thanks to @Nj221102 for the implementation and @MichaelChirico for the suggestion.
+
 ## TRANSLATIONS
 
 1. Fix a typo in a Mandarin translation of an error message that was hiding the actual error message, [#6172](https://github.com/Rdatatable/data.table/issues/6172). Thanks @trafficfan for the report and @MichaelChirico for the fix.
diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw
index 57a348ce15..c98624fd2f 100644
--- a/inst/tests/tests.Rraw
+++ b/inst/tests/tests.Rraw
@@ -5578,8 +5578,8 @@ test(1352.8, DT[,sum(b),by=.(a%%2,c)], DT[,sum(b),by=list(a%%2,c)])
 
 # that :=NULL together with i is now an error
 DT = data.table(a=1:3, b=1:6)
-test(1353.1, DT[2, b:=NULL], error="When deleting columns, i should not be provided")
-test(1353.2, DT[2, c("a","b"):=list(42, NULL)], error="When deleting columns, i should not be provided")
+test(1353.1, DT[2, b:=NULL], error=".*Direct assignment of NULL to a list column item is not supported.*")
+test(1353.2, DT[2, c("a","b"):=list(42, NULL)], error=".*Direct assignment of NULL to a list column item is not supported.*")
 
 # order optimisation caused trouble due to chaining because of 'substitute(x)' usage in [.data.table.
 set.seed(1L)
@@ -7243,7 +7243,7 @@ if (test_bit64) {
 # fix for #1082
 dt1 = data.table(x=rep(c("a","b","c"),each=3), y=c(1,3,6), v=1:9, key=c("x", "y"))
 dt2 = copy(dt1)
-test(1502.1, dt1["a", z := NULL], error="When deleting columns, i should not be provided")
+test(1502.1, dt1["a", z := NULL], error=".*Direct assignment of NULL to a list column item is not supported.*")
 # this shouldn't segfault on 'dt1[...]'
 test(1502.2, dt1["a", z := 42L], dt2["a", z := 42L])
 
@@ -13235,7 +13235,7 @@ test(1946, unique(DT, by="B"), data.table(A=1:3, B=1:3, foo=7:9))
 
 # corruption when deleting a missing column and providing i too, #3089
 DT = data.table(A=1:5)
-test(1947.1, DT[A<0, c('A','B'):=.(NULL, A)], error="When deleting columns, i should not be provided")
+test(1947.1, DT[A<0, c('A','B'):=.(NULL, A)], error=".*Direct assignment of NULL to a list column item is not supported.*")
 test(1947.2, DT, data.table(A=1:5))
 
 ## tests for backticks and spaces in column names of on=, #2931
diff --git a/src/assign.c b/src/assign.c
index a78b9452c4..12ad3e4336 100644
--- a/src/assign.c
+++ b/src/assign.c
@@ -458,7 +458,7 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values)
     coln--;
     SEXP thisvalue = RHS_list_of_columns ? VECTOR_ELT(values, i) : values;
     vlen = length(thisvalue);
-    if (isNull(thisvalue) && !isNull(rows)) error(_("When deleting columns, i should not be provided"));  // #1082, #3089
+    if (isNull(thisvalue) && !isNull(rows)) error(_("Direct assignment of NULL to a list column item is not supported. If you intend to assign NULL to an item in the list column, use list(list(NULL)) in your query."));  // #1082, #3089
     if (coln+1 <= oldncol) colnam = STRING_ELT(names,coln);
     else colnam = STRING_ELT(newcolnames,coln-length(names));
     if (coln+1 <= oldncol && isNull(thisvalue)) continue;  // delete existing column(s) afterwards, near end of this function

From 743cfdd43eeaab3899c65c5a3a276a0391fb74a8 Mon Sep 17 00:00:00 2001
From: nitish jha <nitishjha@nitishs-MacBook-Air.local>
Date: Fri, 2 Aug 2024 02:31:38 +0530
Subject: [PATCH 2/8] updated error

---
 src/assign.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/assign.c b/src/assign.c
index 12ad3e4336..e119056702 100644
--- a/src/assign.c
+++ b/src/assign.c
@@ -458,7 +458,7 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values)
     coln--;
     SEXP thisvalue = RHS_list_of_columns ? VECTOR_ELT(values, i) : values;
     vlen = length(thisvalue);
-    if (isNull(thisvalue) && !isNull(rows)) error(_("Direct assignment of NULL to a list column item is not supported. If you intend to assign NULL to an item in the list column, use list(list(NULL)) in your query."));  // #1082, #3089
+    if (isNull(thisvalue) && !isNull(rows)) error(_("Direct assignment of NULL when `i` is provided is not supported. If you intend to assign NULL to a column, use `NULL` without `i`. For list columns, if you intend to assign NULL to a specific item, use `list(list(NULL))` in your query."));  // #1082, #3089
     if (coln+1 <= oldncol) colnam = STRING_ELT(names,coln);
     else colnam = STRING_ELT(newcolnames,coln-length(names));
     if (coln+1 <= oldncol && isNull(thisvalue)) continue;  // delete existing column(s) afterwards, near end of this function

From e65f62ef397654345cead1334bcd70e3f09c5198 Mon Sep 17 00:00:00 2001
From: nitish jha <nitishjha@nitishs-MacBook-Air.local>
Date: Fri, 2 Aug 2024 02:34:36 +0530
Subject: [PATCH 3/8] updated test

---
 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 c98624fd2f..b2476772b9 100644
--- a/inst/tests/tests.Rraw
+++ b/inst/tests/tests.Rraw
@@ -5578,8 +5578,8 @@ test(1352.8, DT[,sum(b),by=.(a%%2,c)], DT[,sum(b),by=list(a%%2,c)])
 
 # that :=NULL together with i is now an error
 DT = data.table(a=1:3, b=1:6)
-test(1353.1, DT[2, b:=NULL], error=".*Direct assignment of NULL to a list column item is not supported.*")
-test(1353.2, DT[2, c("a","b"):=list(42, NULL)], error=".*Direct assignment of NULL to a list column item is not supported.*")
+test(1353.1, DT[2, b:=NULL], error=".*Direct assignment of NULL when `i` is provided is not supported.*")
+test(1353.2, DT[2, c("a","b"):=list(42, NULL)], error=".*Direct assignment of NULL when `i` is provided is not supported.*")
 
 # order optimisation caused trouble due to chaining because of 'substitute(x)' usage in [.data.table.
 set.seed(1L)
@@ -7243,7 +7243,7 @@ if (test_bit64) {
 # fix for #1082
 dt1 = data.table(x=rep(c("a","b","c"),each=3), y=c(1,3,6), v=1:9, key=c("x", "y"))
 dt2 = copy(dt1)
-test(1502.1, dt1["a", z := NULL], error=".*Direct assignment of NULL to a list column item is not supported.*")
+test(1502.1, dt1["a", z := NULL], error=".*Direct assignment of NULL when `i` is provided is not supported.*")
 # this shouldn't segfault on 'dt1[...]'
 test(1502.2, dt1["a", z := 42L], dt2["a", z := 42L])
 
@@ -13235,7 +13235,7 @@ test(1946, unique(DT, by="B"), data.table(A=1:3, B=1:3, foo=7:9))
 
 # corruption when deleting a missing column and providing i too, #3089
 DT = data.table(A=1:5)
-test(1947.1, DT[A<0, c('A','B'):=.(NULL, A)], error=".*Direct assignment of NULL to a list column item is not supported.*")
+test(1947.1, DT[A<0, c('A','B'):=.(NULL, A)], error=".*Direct assignment of NULL when `i` is provided is not supported.*")
 test(1947.2, DT, data.table(A=1:5))
 
 ## tests for backticks and spaces in column names of on=, #2931

From 1626036fb13cefa19a32b7b1fa0abf225485054f Mon Sep 17 00:00:00 2001
From: nitish jha <nitishjha@nitishs-MacBook-Air.local>
Date: Fri, 2 Aug 2024 08:42:36 +0530
Subject: [PATCH 4/8] removed backticks

---
 inst/tests/tests.Rraw | 8 ++++----
 src/assign.c          | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw
index b2476772b9..ccff0ded93 100644
--- a/inst/tests/tests.Rraw
+++ b/inst/tests/tests.Rraw
@@ -5578,8 +5578,8 @@ test(1352.8, DT[,sum(b),by=.(a%%2,c)], DT[,sum(b),by=list(a%%2,c)])
 
 # that :=NULL together with i is now an error
 DT = data.table(a=1:3, b=1:6)
-test(1353.1, DT[2, b:=NULL], error=".*Direct assignment of NULL when `i` is provided is not supported.*")
-test(1353.2, DT[2, c("a","b"):=list(42, NULL)], error=".*Direct assignment of NULL when `i` is provided is not supported.*")
+test(1353.1, DT[2, b:=NULL], error=".*Direct assignment of NULL when i is provided is not supported.*")
+test(1353.2, DT[2, c("a","b"):=list(42, NULL)], error=".*Direct assignment of NULL when i is provided is not supported.*")
 
 # order optimisation caused trouble due to chaining because of 'substitute(x)' usage in [.data.table.
 set.seed(1L)
@@ -7243,7 +7243,7 @@ if (test_bit64) {
 # fix for #1082
 dt1 = data.table(x=rep(c("a","b","c"),each=3), y=c(1,3,6), v=1:9, key=c("x", "y"))
 dt2 = copy(dt1)
-test(1502.1, dt1["a", z := NULL], error=".*Direct assignment of NULL when `i` is provided is not supported.*")
+test(1502.1, dt1["a", z := NULL], error=".*Direct assignment of NULL when i is provided is not supported.*")
 # this shouldn't segfault on 'dt1[...]'
 test(1502.2, dt1["a", z := 42L], dt2["a", z := 42L])
 
@@ -13235,7 +13235,7 @@ test(1946, unique(DT, by="B"), data.table(A=1:3, B=1:3, foo=7:9))
 
 # corruption when deleting a missing column and providing i too, #3089
 DT = data.table(A=1:5)
-test(1947.1, DT[A<0, c('A','B'):=.(NULL, A)], error=".*Direct assignment of NULL when `i` is provided is not supported.*")
+test(1947.1, DT[A<0, c('A','B'):=.(NULL, A)], error=".*Direct assignment of NULL when i is provided is not supported.*")
 test(1947.2, DT, data.table(A=1:5))
 
 ## tests for backticks and spaces in column names of on=, #2931
diff --git a/src/assign.c b/src/assign.c
index e119056702..46b352b5f4 100644
--- a/src/assign.c
+++ b/src/assign.c
@@ -458,7 +458,7 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values)
     coln--;
     SEXP thisvalue = RHS_list_of_columns ? VECTOR_ELT(values, i) : values;
     vlen = length(thisvalue);
-    if (isNull(thisvalue) && !isNull(rows)) error(_("Direct assignment of NULL when `i` is provided is not supported. If you intend to assign NULL to a column, use `NULL` without `i`. For list columns, if you intend to assign NULL to a specific item, use `list(list(NULL))` in your query."));  // #1082, #3089
+    if (isNull(thisvalue) && !isNull(rows)) error(_("Direct assignment of NULL when i is provided is not supported. If you intend to assign NULL to a column, use `NULL` without i. For list columns, if you intend to assign NULL to a specific item, use list(list(NULL)) in your query."));  // #1082, #3089
     if (coln+1 <= oldncol) colnam = STRING_ELT(names,coln);
     else colnam = STRING_ELT(newcolnames,coln-length(names));
     if (coln+1 <= oldncol && isNull(thisvalue)) continue;  // delete existing column(s) afterwards, near end of this function

From 705990925a4b90225c713a4abc74ec19dbfd94f0 Mon Sep 17 00:00:00 2001
From: nitish jha <nitishjha@nitishs-MacBook-Air.local>
Date: Fri, 2 Aug 2024 08:44:14 +0530
Subject: [PATCH 5/8] small fix

---
 src/assign.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/assign.c b/src/assign.c
index 46b352b5f4..35974f7179 100644
--- a/src/assign.c
+++ b/src/assign.c
@@ -458,7 +458,7 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values)
     coln--;
     SEXP thisvalue = RHS_list_of_columns ? VECTOR_ELT(values, i) : values;
     vlen = length(thisvalue);
-    if (isNull(thisvalue) && !isNull(rows)) error(_("Direct assignment of NULL when i is provided is not supported. If you intend to assign NULL to a column, use `NULL` without i. For list columns, if you intend to assign NULL to a specific item, use list(list(NULL)) in your query."));  // #1082, #3089
+    if (isNull(thisvalue) && !isNull(rows)) error(_("Direct assignment of NULL when i is provided is not supported. If you intend to assign NULL to a column, use NULL without i. For list columns, if you intend to assign NULL to a specific item, use list(list(NULL)) in your query."));  // #1082, #3089
     if (coln+1 <= oldncol) colnam = STRING_ELT(names,coln);
     else colnam = STRING_ELT(newcolnames,coln-length(names));
     if (coln+1 <= oldncol && isNull(thisvalue)) continue;  // delete existing column(s) afterwards, near end of this function

From f875a8e74f064ebbb48a9f75fc419e9a9a797510 Mon Sep 17 00:00:00 2001
From: Michael Chirico <michaelchirico4@gmail.com>
Date: Sat, 7 Sep 2024 10:44:33 -0700
Subject: [PATCH 6/8] re-site NEWS entry

---
 NEWS.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index 59cebb9109..5f068ea633 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -69,6 +69,8 @@ rowwiseDT(
 
 5. Some grouping operations run much faster under `verbose=TRUE`, [#6286](https://github.com/Rdatatable/data.table/issues/6286). Thanks @joshhwuu for the report and fix. This overhead was not present on Windows. As a rule, users should expect `verbose=TRUE` operations to run more slowly, as extra statistics might be calculated as part of the report; here was a case where the overhead was particularly high and the fix was particularly easy.
 
+6. `set()` and `:=` now provide an improved error message when attempting to directly assign `NULL` to a list column item. The new message guides users to use `list(list(NULL))` instead, enhancing clarity and preventing common mistakes. Previously, this operation would throw a generic error, but now the message is more informative and helpful. Thanks to @Nj221102 for the implementation and @MichaelChirico for the suggestion.
+
 # data.table [v1.16.0](https://github.com/Rdatatable/data.table/milestone/30)  (25 August 2024)
 
 ## BREAKING CHANGES
@@ -272,8 +274,6 @@ rowwiseDT(
 
 19. `?dcast` has always required `fun.aggregate` to return a single value, and when `fill=NULL`, `dcast` would indeed error if a vector with `length!=1` was returned, but an undefined result was silently returned for non-`NULL` fill. Now `dcast()` will additionally warn that this is undefined behavior when fill is not `NULL`, [#6032](https://github.com/Rdatatable/data.table/issues/6032). In particular, this will warn for `fun.aggregate=identity`, which was observed in several revdeps. We may change this to an error in a future release, so revdeps should fix their code as soon as possible. Thanks to @tdhock for the PR, and @MichaelChirico for analysis of GitHub revdeps.
 
-25. `set()` and `:=` now provide an improved error message when attempting to directly assign `NULL` to a list column item. The new message guides users to use `list(list(NULL))` instead, enhancing clarity and preventing common mistakes. Previously, this operation would throw a generic error, but now the message is more informative and helpful. Thanks to @Nj221102 for the implementation and @MichaelChirico for the suggestion.
-
 ## TRANSLATIONS
 
 1. Fix a typo in a Mandarin translation of an error message that was hiding the actual error message, [#6172](https://github.com/Rdatatable/data.table/issues/6172). Thanks @trafficfan for the report and @MichaelChirico for the fix.

From f955518be3ae504c96718a1490923a5e772c5329 Mon Sep 17 00:00:00 2001
From: Michael Chirico <michaelchirico4@gmail.com>
Date: Sat, 7 Sep 2024 18:31:04 -0700
Subject: [PATCH 7/8] Improve NEWS wording

---
 NEWS.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/NEWS.md b/NEWS.md
index 5f068ea633..32097b4ad8 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -69,7 +69,7 @@ rowwiseDT(
 
 5. Some grouping operations run much faster under `verbose=TRUE`, [#6286](https://github.com/Rdatatable/data.table/issues/6286). Thanks @joshhwuu for the report and fix. This overhead was not present on Windows. As a rule, users should expect `verbose=TRUE` operations to run more slowly, as extra statistics might be calculated as part of the report; here was a case where the overhead was particularly high and the fix was particularly easy.
 
-6. `set()` and `:=` now provide an improved error message when attempting to directly assign `NULL` to a list column item. The new message guides users to use `list(list(NULL))` instead, enhancing clarity and preventing common mistakes. Previously, this operation would throw a generic error, but now the message is more informative and helpful. Thanks to @Nj221102 for the implementation and @MichaelChirico for the suggestion.
+6. `set()` and `:=` now provide some extra guidance for common incorrect approaches to assigning `NULL` to some rows of a list column. The correct way is to put `list(list(NULL))` on the RHS of `:=` (or `.(.(NULL))` for short). Thanks to @MichaelChirico for the suggestion and @Nj221102 for the implementation.
 
 # data.table [v1.16.0](https://github.com/Rdatatable/data.table/milestone/30)  (25 August 2024)
 

From c5947a02c4fcc4358a1f24da16c24df81ff8b5a2 Mon Sep 17 00:00:00 2001
From: Michael Chirico <michaelchirico4@gmail.com>
Date: Sat, 7 Sep 2024 18:31:37 -0700
Subject: [PATCH 8/8] Improve error preciseness, add tests

---
 inst/tests/tests.Rraw | 15 ++++++++++-----
 src/assign.c          | 10 ++++++++--
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw
index 38a16c45f7..a6bbdbf862 100644
--- a/inst/tests/tests.Rraw
+++ b/inst/tests/tests.Rraw
@@ -5580,9 +5580,14 @@ test(1352.7, DT[,sum(b),by=.(Grp=a%%2)], DT[,sum(b),by=list(Grp=a%%2)])
 test(1352.8, DT[,sum(b),by=.(a%%2,c)], DT[,sum(b),by=list(a%%2,c)])
 
 # that :=NULL together with i is now an error
-DT = data.table(a=1:3, b=1:6)
-test(1353.1, DT[2, b:=NULL], error=".*Direct assignment of NULL when i is provided is not supported.*")
-test(1353.2, DT[2, c("a","b"):=list(42, NULL)], error=".*Direct assignment of NULL when i is provided is not supported.*")
+DT = data.table(a=1:3, b=1:6, c=list(7, 8, 9, 8, 7, 6))
+test(1353.1, DT[2, b:=NULL], error="When deleting columns, i should not be provided")
+test(1353.2, DT[2, c("a","b"):=list(42, NULL)], error="When deleting columns, i should not be provided")
+# #5526: friendlier error nudging to the correct way to sub-assign NULL to list columns
+test(1353.3, DT[2, c := NULL], error="Invalid attempt to delete a list column.*did you intend to add NULL")
+test(1353.4, DT[2, c := .(NULL)], error="Invalid attempt to delete a list column.*did you intend to add NULL")
+test(1353.5, DT[2, `:=`(b=2, c=NULL)], error="Invalid attempt to delete a list column.*did you intend to add NULL")
+test(1353.6, DT[2, d := NULL], error="Doubly-invalid attempt to delete a non-existent column while also providing i")
 
 # order optimisation caused trouble due to chaining because of 'substitute(x)' usage in [.data.table.
 set.seed(1L)
@@ -7246,7 +7251,7 @@ if (test_bit64) {
 # fix for #1082
 dt1 = data.table(x=rep(c("a","b","c"),each=3), y=c(1,3,6), v=1:9, key=c("x", "y"))
 dt2 = copy(dt1)
-test(1502.1, dt1["a", z := NULL], error=".*Direct assignment of NULL when i is provided is not supported.*")
+test(1502.1, dt1["a", z := NULL], error="Doubly-invalid attempt to delete a non-existent column while also providing i")
 # this shouldn't segfault on 'dt1[...]'
 test(1502.2, dt1["a", z := 42L], dt2["a", z := 42L])
 
@@ -13241,7 +13246,7 @@ test(1946, unique(DT, by="B"), data.table(A=1:3, B=1:3, foo=7:9))
 
 # corruption when deleting a missing column and providing i too, #3089
 DT = data.table(A=1:5)
-test(1947.1, DT[A<0, c('A','B'):=.(NULL, A)], error=".*Direct assignment of NULL when i is provided is not supported.*")
+test(1947.1, DT[A<0, c('A','B'):=.(NULL, A)], error="When deleting columns, i should not be provided")
 test(1947.2, DT, data.table(A=1:5))
 
 ## tests for backticks and spaces in column names of on=, #2931
diff --git a/src/assign.c b/src/assign.c
index aba765d3fb..e12d6f4496 100644
--- a/src/assign.c
+++ b/src/assign.c
@@ -467,11 +467,17 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values)
     }
     coln--;
     SEXP thisvalue = RHS_list_of_columns ? VECTOR_ELT(values, i) : values;
-    vlen = length(thisvalue);
-    if (isNull(thisvalue) && !isNull(rows)) error(_("Direct assignment of NULL when i is provided is not supported. If you intend to assign NULL to a column, use NULL without i. For list columns, if you intend to assign NULL to a specific item, use list(list(NULL)) in your query."));  // #1082, #3089
+    if (isNull(thisvalue) && !isNull(rows)) {
+      if (coln >= oldncol)
+        error(_("Doubly-invalid attempt to delete a non-existent column while also providing i"));
+      if (TYPEOF(VECTOR_ELT(dt, coln)) == VECSXP)
+        error(_("Invalid attempt to delete a list column while also providing i; did you intend to add NULL to those rows instead? If so, use list_col := list(list(NULL)).")); // #5526
+      error(_("When deleting columns, i should not be provided"));  // #1082, #3089
+    }
     if (coln+1 <= oldncol) colnam = STRING_ELT(names,coln);
     else colnam = STRING_ELT(newcolnames,coln-length(names));
     if (coln+1 <= oldncol && isNull(thisvalue)) continue;  // delete existing column(s) afterwards, near end of this function
+    vlen = length(thisvalue);
     //if (vlen<1 && nrow>0) {
     if (coln+1 <= oldncol && nrow>0 && vlen<1 && numToDo>0) { // numToDo > 0 fixes #2829, see test 1911
       error(_("RHS of assignment to existing column '%s' is zero length but not NULL. If you intend to delete the column use NULL. Otherwise, the RHS must have length > 0; e.g., NA_integer_. If you are trying to change the column type to be an empty list column then, as with all column type changes, provide a full length RHS vector such as vector('list',nrow(DT)); i.e., 'plonk' in the new column."), CHAR(STRING_ELT(names,coln)));
