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]


Reply via email to