paleolimbot commented on code in PR #19706:
URL: https://github.com/apache/arrow/pull/19706#discussion_r1067137683
##########
r/tests/testthat/test-expression.R:
##########
@@ -76,6 +76,16 @@ test_that("Field reference expression schemas and types", {
expect_equal(x$type(), int32())
})
+test_that("Nested field refs", {
+ x <- Expression$field_ref("x")
+ nested <- x$y
+ expect_r6_class(nested, "Expression")
+ expect_r6_class(x[["y"]], "Expression")
+ expect_r6_class(nested$z, "Expression")
+ # Should this instead be NULL?
Review Comment:
An error seems safer for now?
##########
r/src/expression.cpp:
##########
@@ -79,6 +110,7 @@ std::shared_ptr<compute::Expression> compute___expr__scalar(
// [[arrow::export]]
std::string compute___expr__ToString(const
std::shared_ptr<compute::Expression>& x) {
+ // TODO: something different if is field ref and IsNested?
return x->ToString();
Review Comment:
Is the string representation awful for a nested reference? (Or do we use it
to implicitly do stuff?) From the lack of failing tests here, it seems like we
don't do a lot of testing of expression objects for whether or not they are
field references.
##########
r/tests/testthat/test-dplyr-query.R:
##########
@@ -714,3 +714,48 @@ test_that("Scalars in expressions match the type of the
field, if possible", {
collect()
expect_equal(result$tpc_h_1, result$as_dbl)
})
+
+test_that("Can use nested field refs", {
+ nested_data <- tibble(int = 1:5, df_col = tibble(a = 6:10, b = 11:15))
+
+ compare_dplyr_binding(
+ .input %>%
+ mutate(
+ nested = df_col$a,
+ times2 = df_col$a * 2
+ ) %>%
+ filter(nested > 7) %>%
+ collect(),
+ nested_data
+ )
+
+ compare_dplyr_binding(
+ .input %>%
+ mutate(
+ nested = df_col$a,
+ times2 = df_col$a * 2
+ ) %>%
+ filter(nested > 7) %>%
+ summarize(sum(times2)) %>%
+ collect(),
+ nested_data
+ )
+
+ # Now with Dataset
Review Comment:
Is there a difference in behaviour that would be expected with Dataset? (If
so, maybe add a note to that comment and if not, maybe remove it?)
##########
r/src/expression.cpp:
##########
@@ -19,7 +19,7 @@
#include <arrow/compute/api_scalar.h>
#include <arrow/compute/exec/expression.h>
-
+#include <iostream>
Review Comment:
Is this addition intentional?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]