[GitHub] [arrow] nealrichardson commented on a change in pull request #9805: ARROW-12093: [R] Throw helpful errors on bad object types in dplyr expressions [WIP]

[GitBox <gi...@apache.org>]

nealrichardson commented on a change in pull request #9805:
URL: https://github.com/apache/arrow/pull/9805#discussion_r601872131



##########
File path: r/R/dplyr.R
##########
@@ -279,6 +280,19 @@ arrow_eval <- function (expr, mask) {
     }
     invisible(structure(msg, class = "try-error", condition = e))
   })
+  # Check that the evaluated expression has one of the expected classes or types
+  if (!inherits(out, c("array_expression", "Expression", "try-error")) &&
+      !typeof(out) %in% c("logical", "integer", "double", "character", "NULL")) {

Review comment:
       I think this is too restrictive--what about "list", "raw", etc., which can convert to Arrow objects? What if the input is an `Array`?
   
   Another approach might be to use a blocklist and only block certain cases (like the `function` you ran into).
   
   I'm also not confident that `arrow_eval` is the right place for this. It very well might be, but maybe there are applications of `arrow_eval` that would, for example, return a function or an environment.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org