From 3d9e7ebd4872d2a5d24f0905536ef18244a76b4f Mon Sep 17 00:00:00 2001
From: Michael Chirico <michael.chirico@grabtaxi.com>
Date: Tue, 3 Sep 2019 01:06:29 +0800
Subject: [PATCH] rbindlist works for expression column

---
 NEWS.md               | 2 ++
 inst/tests/tests.Rraw | 4 ++++
 src/assign.c          | 9 ++++++---
 src/rbindlist.c       | 7 ++++---
 4 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index 65dfb8f3a8..b185641825 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -259,6 +259,8 @@
 
 38. `rbindlist` now returns correct idcols for lists with different length vectors, [#3785](https://github.com/Rdatatable/data.table/issues/3785), [#3786](https://github.com/Rdatatable/data.table/pull/3786). Thanks to @shrektan for the report and fix.
 
+39. `rbindlist` successfully combines `expression`-type columns, [#546](https://github.com/Rdatatable/data.table/issues/546). Thanks @jangorecki for the report.
+
 #### NOTES
 
 1. `rbindlist`'s `use.names="check"` now emits its message for automatic column names (`"V[0-9]+"`) too, [#3484](https://github.com/Rdatatable/data.table/pull/3484). See news item 5 of v1.12.2 below.
diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw
index 4582e25a67..1caf8c3a54 100644
--- a/inst/tests/tests.Rraw
+++ b/inst/tests/tests.Rraw
@@ -15864,6 +15864,10 @@ test(2095, any(grepl('override', capture.output(dcast(DT, a~b, fun.aggregate=len
 # gmean intermediate can overflow integers without warning, #986
 test(2096, data.table(a=c(1L,1L), v=c(2e9L, 2e9L))[, mean(v), a], data.table(a=1L, V1=2e9))
 
+# expression columns in rbindlist #546
+a <- data.table(c1 = 1, c2 = 'asd', c3 = expression(as.character(Sys.time())))
+b <- data.table(c1 = 3, c2 = 'qwe', c3 = expression(as.character(Sys.time()+5)))
+test(2097, rbind(a,b)$c3, expression(as.character(Sys.time()), as.character(Sys.time()+5)))
 
 ###################################
 #  Add new tests above this line  #
diff --git a/src/assign.c b/src/assign.c
index 42a4022b27..bb8793ac70 100644
--- a/src/assign.c
+++ b/src/assign.c
@@ -907,14 +907,17 @@ const char *memrecycle(SEXP target, SEXP where, int start, int len, SEXP source)
         for (int i=0; i<len; i++) SET_STRING_ELT(target, start+i, val[i]);
       }
       break;
-    case VECSXP :
-      if (TYPEOF(source)==VECSXP && len==slen) {
+    // #546 -- expression columns can be handled as list columns
+    case VECSXP : case EXPRSXP : {
+      bool source_match=TYPEOF(source)==TYPEOF(target);
+      if (source_match && len==slen) {
         for (int i=0; i<len; i++) SET_VECTOR_ELT(target, start+i, VECTOR_ELT(source, i));
       } else {
-        const SEXP val = TYPEOF(source)==VECSXP ? VECTOR_ELT(source, 0) : source;
+        const SEXP val = source_match ? VECTOR_ELT(source, 0) : source;
         for (int i=0; i<len; i++) SET_VECTOR_ELT(target, start+i, val);
       }
       break;
+    }
     default :
       error("Unsupported type in assign.c:memrecycle '%s' (no where)", type2char(TYPEOF(target)));  // # nocov
     }
diff --git a/src/rbindlist.c b/src/rbindlist.c
index 80426a5ac3..8ef48bbf6a 100644
--- a/src/rbindlist.c
+++ b/src/rbindlist.c
@@ -289,7 +289,8 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg)
       }
       SEXP thisCol = VECTOR_ELT(li, w);
       int thisType = TYPEOF(thisCol);
-      if (TYPEORDER(thisType)>TYPEORDER(maxType)) maxType=thisType;
+      // Use >= for #546 -- TYPEORDER=0 for both LGLSXP and EXPRSXP (but also NULL)
+      if (TYPEORDER(thisType)>=TYPEORDER(maxType) && !isNull(thisCol)) maxType=thisType;
       if (isFactor(thisCol)) {
         if (isNull(getAttrib(thisCol,R_LevelsSymbol))) error("Column %d of item %d has type 'factor' but has no levels; i.e. malformed.", w+1, i+1);
         factor = true;
@@ -480,9 +481,9 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg)
           writeNA(target, ansloc, thisnrow);  // writeNA is integer64 aware and writes INT64_MIN
         } else {
           bool coerced = false;
-          if (TYPEOF(target)==VECSXP && TYPEOF(thisCol)!=VECSXP) {
+          if ((TYPEOF(target)==VECSXP || TYPEOF(target)==EXPRSXP) && TYPEOF(thisCol)!=TYPEOF(target)) {
             // do an as.list() on the atomic column; #3528
-            thisCol = PROTECT(coerceVector(thisCol, VECSXP));
+            thisCol = PROTECT(coerceVector(thisCol, TYPEOF(target)));
             coerced = true;
           } // else coerces if needed within memrecycle; possibly with a no-alloc direct coerce
           const char *ret = memrecycle(target, R_NilValue, ansloc, thisnrow, thisCol);