[GitHub] [arrow] ianmcook commented on a diff in pull request #33917: GH-33892: [R] Map dplyr::n() to count_all kernel

["ianmcook (via GitHub)" <gi...@apache.org>]

ianmcook commented on code in PR #33917:
URL: https://github.com/apache/arrow/pull/33917#discussion_r1095180917


##########
r/R/query-engine.R:
##########
@@ -101,7 +101,11 @@ ExecPlan <- R6Class("ExecPlan",
         # plus group_by_vars (last)
         # TODO: validate that none of names(aggregations) are the same as names(group_by_vars)
         # dplyr does not error on this but the result it gives isn't great
-        node <- node$Project(summarize_projection(.data))
+        projection <- summarize_projection(.data)
+        # skip projection if no grouping and all aggregate functions are nullary

Review Comment:
   In general we want to avoid adding unnecessary projections to the ExecPlan. This is a case where we can do that.



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

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