You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/07/01 18:41:47 UTC

[GitHub] [arrow] pitrou opened a new pull request #7612: ARROW-7011: [C++] Implement casts from float/double to decimal

pitrou opened a new pull request #7612:
URL: https://github.com/apache/arrow/pull/7612


   Also naturally available in Python using the Array.cast() method.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] wesm commented on a change in pull request #7612: ARROW-7011: [C++] Implement casts from float/double to decimal

Posted by GitBox <gi...@apache.org>.
wesm commented on a change in pull request #7612:
URL: https://github.com/apache/arrow/pull/7612#discussion_r449248816



##########
File path: cpp/src/arrow/compute/kernels/scalar_cast_numeric.cc
##########
@@ -467,6 +467,51 @@ struct CastFunctor<Decimal128Type, Decimal128Type> {
   }
 };
 
+// ----------------------------------------------------------------------
+// Real to decimal
+
+template <bool AllowTruncate>
+struct RealToDecimal {
+  template <typename OUT, typename RealType>
+  Decimal128 Call(KernelContext* ctx, RealType val) const {
+    auto result = Decimal128::FromReal(val, out_precision_, out_scale_);
+    if (ARROW_PREDICT_FALSE(!result.ok())) {
+      if (!AllowTruncate) {
+        ctx->SetStatus(result.status());
+      }
+      return Decimal128();  // Zero
+    } else {
+      return *std::move(result);
+    }
+  }
+
+  int32_t out_scale_, out_precision_;
+};
+
+using SafeRealToDecimal = RealToDecimal<true>;
+using UnsafeRealToDecimal = RealToDecimal<false>;

Review comment:
       I'm not sure the performance benefit of pruning the `AllowTruncate` branch merits the 2x code size -- what do you think about making this flag just a member of the functor?




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] wesm closed pull request #7612: ARROW-7011: [C++] Implement casts from float/double to decimal

Posted by GitBox <gi...@apache.org>.
wesm closed pull request #7612:
URL: https://github.com/apache/arrow/pull/7612


   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] wesm commented on pull request #7612: ARROW-7011: [C++] Implement casts from float/double to decimal

Posted by GitBox <gi...@apache.org>.
wesm commented on pull request #7612:
URL: https://github.com/apache/arrow/pull/7612#issuecomment-653223898


   Ugh there's that pesky flake again https://github.com/apache/arrow/pull/7612/checks?check_run_id=832227898
   
   @kszucs did we determine whether it's feasible to get backtraces on macOS in GitHub Actions?


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] pitrou commented on a change in pull request #7612: ARROW-7011: [C++] Implement casts from float/double to decimal

Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #7612:
URL: https://github.com/apache/arrow/pull/7612#discussion_r449258150



##########
File path: cpp/src/arrow/compute/kernels/scalar_cast_numeric.cc
##########
@@ -467,6 +467,51 @@ struct CastFunctor<Decimal128Type, Decimal128Type> {
   }
 };
 
+// ----------------------------------------------------------------------
+// Real to decimal
+
+template <bool AllowTruncate>
+struct RealToDecimal {
+  template <typename OUT, typename RealType>
+  Decimal128 Call(KernelContext* ctx, RealType val) const {
+    auto result = Decimal128::FromReal(val, out_precision_, out_scale_);
+    if (ARROW_PREDICT_FALSE(!result.ok())) {
+      if (!AllowTruncate) {
+        ctx->SetStatus(result.status());
+      }
+      return Decimal128();  // Zero
+    } else {
+      return *std::move(result);
+    }
+  }
+
+  int32_t out_scale_, out_precision_;
+};
+
+using SafeRealToDecimal = RealToDecimal<true>;
+using UnsafeRealToDecimal = RealToDecimal<false>;

Review comment:
       Hum, yes, sounds fine. I initially thought the code would be more different, but it doesn't really make sense to generate two codepaths now.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] wesm commented on pull request #7612: ARROW-7011: [C++] Implement casts from float/double to decimal

Posted by GitBox <gi...@apache.org>.
wesm commented on pull request #7612:
URL: https://github.com/apache/arrow/pull/7612#issuecomment-653232883


   +1


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] github-actions[bot] commented on pull request #7612: ARROW-7011: [C++] Implement casts from float/double to decimal

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #7612:
URL: https://github.com/apache/arrow/pull/7612#issuecomment-652589053


   https://issues.apache.org/jira/browse/ARROW-7011


----------------------------------------------------------------
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:
users@infra.apache.org