Skip to content

Commit

Permalink
ARROW-17639: [R] infer_type() fails for lists where the first element…
Browse files Browse the repository at this point in the history
… is NULL (apache#14062)

This PR updates the internals of type inference on lists; instead of looking at the type of the first element, we instead iterate through all elements until we find a non-null element.

Authored-by: Nic Crane <thisisnic@gmail.com>
Signed-off-by: Nic Crane <thisisnic@gmail.com>
  • Loading branch information
thisisnic authored and fatemehp committed Oct 17, 2022
1 parent a2e67e3 commit 63ff062
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 2 deletions.
11 changes: 9 additions & 2 deletions r/src/type_infer.cpp
Expand Up @@ -165,8 +165,13 @@ std::shared_ptr<arrow::DataType> InferArrowTypeFromVector<VECSXP>(SEXP x) {
cpp11::stop(
"Requires at least one element to infer the values' type of a list vector");
}

ptype = VECTOR_ELT(x, 0);
// Iterate through the vector until we get a non-null result
for (R_xlen_t i = 0; i < XLENGTH(x); i++) {
ptype = VECTOR_ELT(x, i);
if (!Rf_isNull(ptype)) {
break;
}
}
}

return arrow::list(InferArrowType(ptype));
Expand Down Expand Up @@ -198,6 +203,8 @@ std::shared_ptr<arrow::DataType> InferArrowType(SEXP x) {
return InferArrowTypeFromVector<STRSXP>(x);
case VECSXP:
return InferArrowTypeFromVector<VECSXP>(x);
case NILSXP:
return null();
default:
cpp11::stop("Cannot infer type from vector");
}
Expand Down
16 changes: 16 additions & 0 deletions r/tests/testthat/test-type.R
Expand Up @@ -293,3 +293,19 @@ test_that("type() is deprecated", {
)
expect_equal(a_type, a$type)
})

test_that("infer_type() infers type for lists starting with NULL - ARROW-17639", {
null_start_list <- list(NULL, c(2, 3), c(4, 5))

expect_equal(
infer_type(null_start_list),
list_of(float64())
)

totally_null_list <- list(NULL, NULL, NULL)

expect_equal(
infer_type(totally_null_list),
list_of(null())
)
})

0 comments on commit 63ff062

Please sign in to comment.