zanmato1984 commented on code in PR #40237:
URL: https://github.com/apache/arrow/pull/40237#discussion_r1576604124


##########
cpp/src/arrow/scalar.cc:
##########
@@ -969,58 +1058,72 @@ std::shared_ptr<Buffer> FormatToBuffer(Formatter&& 
formatter, const ScalarType&
 }
 
 // error fallback
-Status CastImpl(const Scalar& from, Scalar* to) {
+template <typename To>
+Result<std::shared_ptr<Scalar>> CastImpl(const Scalar& from,
+                                         std::shared_ptr<DataType> to_type) {
   return Status::NotImplemented("casting scalars of type ", *from.type, " to 
type ",
-                                *to->type);
+                                *to_type);
 }
 
 // numeric to numeric
-template <typename From, typename To>
-Status CastImpl(const NumericScalar<From>& from, NumericScalar<To>* to) {
-  to->value = static_cast<typename To::c_type>(from.value);
-  return Status::OK();
+template <typename To, typename From>
+enable_if_number<To, Result<std::shared_ptr<Scalar>>> CastImpl(
+    const NumericScalar<From>& from, std::shared_ptr<DataType> to_type) {
+  using ToScalar = typename TypeTraits<To>::ScalarType;
+  return std::make_shared<ToScalar>(static_cast<typename 
To::c_type>(from.value),
+                                    std::move(to_type));
 }
 
 // numeric to boolean
-template <typename T>
-Status CastImpl(const NumericScalar<T>& from, BooleanScalar* to) {
-  constexpr auto zero = static_cast<typename T::c_type>(0);
-  to->value = from.value != zero;
-  return Status::OK();
+template <typename To, typename From>
+enable_if_boolean<To, Result<std::shared_ptr<Scalar>>> CastImpl(
+    const NumericScalar<From>& from, std::shared_ptr<DataType> to_type) {
+  constexpr auto zero = static_cast<typename From::c_type>(0);
+  return std::make_shared<BooleanScalar>(from.value != zero, 
std::move(to_type));
 }
 
 // boolean to numeric
-template <typename T>
-Status CastImpl(const BooleanScalar& from, NumericScalar<T>* to) {
-  to->value = static_cast<typename T::c_type>(from.value);
-  return Status::OK();
+template <typename To>
+enable_if_number<To, Result<std::shared_ptr<Scalar>>> CastImpl(
+    const BooleanScalar& from, std::shared_ptr<DataType> to_type) {
+  using ToScalar = typename TypeTraits<To>::ScalarType;
+  return std::make_shared<ToScalar>(static_cast<typename 
To::c_type>(from.value),
+                                    std::move(to_type));
 }
 
 // numeric to temporal
-template <typename From, typename To>
+template <typename To, typename From>
 typename std::enable_if<std::is_base_of<TemporalType, To>::value &&
                             !std::is_same<DayTimeIntervalType, To>::value &&
                             !std::is_same<MonthDayNanoIntervalType, To>::value,
-                        Status>::type
-CastImpl(const NumericScalar<From>& from, TemporalScalar<To>* to) {
-  to->value = static_cast<typename To::c_type>(from.value);
-  return Status::OK();
+                        Result<std::shared_ptr<Scalar>>>::type
+CastImpl(const NumericScalar<From>& from, std::shared_ptr<DataType> to_type) {
+  using ToScalar = typename TypeTraits<To>::ScalarType;
+  return std::make_shared<ToScalar>(static_cast<typename 
To::c_type>(from.value),
+                                    std::move(to_type));
 }
 
 // temporal to numeric
-template <typename From, typename To>
-typename std::enable_if<std::is_base_of<TemporalType, From>::value &&
+template <typename To, typename From>

Review Comment:
   Not really. As this PR makes `BaseBinaryScalar::value` immutable, it is 
required to accordingly change the `CastImplVisitior` to create the scalar with 
a calculated `value` instead of create-scalar-then-assign the `value`. To do 
this, all `CastImpl` specializations should accept an explicit `To` type 
parameter as it is not able to deduce the type from an input argument. And this 
specific line of change is to make the template parameter have the same shape 
as others.



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to