nealrichardson commented on a change in pull request #8341:
URL: https://github.com/apache/arrow/pull/8341#discussion_r500365879
##########
File path: r/src/array_to_vector.cpp
##########
@@ -1068,8 +1080,9 @@ std::shared_ptr<Converter> Converter::Make(const
std::shared_ptr<DataType>& type
return
std::make_shared<arrow::r::Converter_Timestamp<int64_t>>(std::move(arrays));
case Type::INT64:
- // Prefer integer if it fits
- if (ArraysCanFitInteger(arrays)) {
+ // Prefer integer if it fits, unless option arrow.int64_auto_downcast is
`false`
+ if (ArraysCanFitInteger(arrays) &&
+ GetBoolOption("arrow.int64_auto_downcast", true)) {
Review comment:
Your other comment said you were going with `arrow.int64_downcast` but
that's not what is here. I'm ok either way though `arrow.int64_downcast` is
more concise (and the "auto" behavior is really about what the default value
is).
##########
File path: r/src/array_to_vector.cpp
##########
@@ -1068,8 +1080,9 @@ std::shared_ptr<Converter> Converter::Make(const
std::shared_ptr<DataType>& type
return
std::make_shared<arrow::r::Converter_Timestamp<int64_t>>(std::move(arrays));
case Type::INT64:
- // Prefer integer if it fits
- if (ArraysCanFitInteger(arrays)) {
+ // Prefer integer if it fits, unless option arrow.int64_auto_downcast is
`false`
+ if (ArraysCanFitInteger(arrays) &&
Review comment:
Minor thought: should we switch the order of these checks? I have to
imagine that GetBoolOption would be faster than doing bounds checking on the
full array.
##########
File path: r/src/array_to_vector.cpp
##########
@@ -960,6 +960,18 @@ bool ArraysCanFitInteger(ArrayVector arrays) {
return all_can_fit;
}
+bool GetBoolOption(const std::string& name, bool default_) {
+ SEXP getOption = Rf_install("getOption");
+ cpp11::sexp call = Rf_lang2(getOption, Rf_mkString(name.c_str()));
+ cpp11::sexp res = Rf_eval(call, R_BaseEnv);
+ Rf_PrintValue(res);
Review comment:
Is this a debugging print statement you meant to remove?
```suggestion
```
----------------------------------------------------------------
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:
[email protected]