bkietz commented on a change in pull request #7279:
URL: https://github.com/apache/arrow/pull/7279#discussion_r431416935
##########
File path: r/src/expression.cpp
##########
@@ -104,57 +104,9 @@ std::shared_ptr<ds::Expression> dataset___expr__is_valid(
// [[arrow::export]]
std::shared_ptr<ds::Expression> dataset___expr__scalar(SEXP x) {
- switch (TYPEOF(x)) {
- case NILSXP:
- return ds::scalar(std::make_shared<arrow::NullScalar>());
- case LGLSXP:
- return ds::scalar(Rf_asLogical(x));
- case REALSXP:
- if (Rf_inherits(x, "Date")) {
- return ds::scalar(std::make_shared<arrow::Date32Scalar>(REAL(x)[0]));
- } else if (Rf_inherits(x, "POSIXct")) {
- return ds::scalar(std::make_shared<arrow::TimestampScalar>(
- REAL(x)[0], arrow::timestamp(arrow::TimeUnit::SECOND)));
- } else if (Rf_inherits(x, "integer64")) {
- int64_t value = *reinterpret_cast<int64_t*>(REAL(x));
- return ds::scalar(value);
- } else if (Rf_inherits(x, "difftime")) {
- int multiplier = 0;
- // TODO: shared with TimeConverter<> in array_from_vector.cpp
- std::string unit(CHAR(STRING_ELT(Rf_getAttrib(x,
arrow::r::symbols::units), 0)));
- if (unit == "secs") {
- multiplier = 1;
- } else if (unit == "mins") {
- multiplier = 60;
- } else if (unit == "hours") {
- multiplier = 3600;
- } else if (unit == "days") {
- multiplier = 86400;
- } else if (unit == "weeks") {
- multiplier = 604800;
- } else {
- Rcpp::stop("unknown difftime unit");
- }
- return ds::scalar(std::make_shared<arrow::Time32Scalar>(
- static_cast<int>(REAL(x)[0] * multiplier),
- arrow::time32(arrow::TimeUnit::SECOND)));
- }
- return ds::scalar(Rf_asReal(x));
- case INTSXP:
- if (Rf_inherits(x, "factor")) {
- // TODO: This does not use the actual value, just the levels
- auto type = arrow::r::InferArrowTypeFromFactor(x);
- return ds::scalar(std::make_shared<arrow::DictionaryScalar>(type));
- }
- return ds::scalar(Rf_asInteger(x));
- case STRSXP:
- return ds::scalar(CHAR(STRING_ELT(x, 0)));
- default:
- Rcpp::stop(
- tfm::format("R object of type %s not supported",
Rf_type2char(TYPEOF(x))));
- }
-
- return nullptr;
+ // defined in scalar.cpp
+ std::shared_ptr<arrow::Scalar> Scalar__create(SEXP x);
Review comment:
Adding `Scalar__create` to the header seems like overkill until it's
used by more than just this function, but I can move it for consistency if
preferred
----------------------------------------------------------------
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:
us...@infra.apache.org
