[arrow] branch main updated: GH-34644: [C++] Prefer unsafe casting by default in Substrait (#34645)

[icexelloss]

This is an automated email from the ASF dual-hosted git repository.

icexelloss pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/main by this push:
     new e5e33395c9 GH-34644: [C++] Prefer unsafe casting by default in 
Substrait  (#34645)
e5e33395c9 is described below

commit e5e33395c9ec7eb8abeb2decf040de72fb40b39e
Author: Weston Pace <weston.p...@gmail.com>
AuthorDate: Thu Mar 23 11:09:49 2023 -0700

    GH-34644: [C++] Prefer unsafe casting by default in Substrait  (#34645)
    
    ### Rationale for this change
    
    See issue
    
    ### What changes are included in this PR?
    
    Changes the default cast in Substrait to an unsafe cast.
    
    ### Are these changes tested?
    
    Yes.
    
    ### Are there any user-facing changes?
    
    Yes
    
    BREAKING CHANGE: The default Substrait behavior has changed slightly.  
However, we believe this to be more in line with user expectation.
    * Closes: #34644
    
    Authored-by: Weston Pace <weston.p...@gmail.com>
    Signed-off-by: Li Jin <ice.xell...@gmail.com>
---
 cpp/src/arrow/engine/substrait/expression_internal.cc |  5 +++--
 cpp/src/arrow/engine/substrait/serde_test.cc          | 13 +++++++++++++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/cpp/src/arrow/engine/substrait/expression_internal.cc 
b/cpp/src/arrow/engine/substrait/expression_internal.cc
index 5f9fec733f..e1223a5132 100644
--- a/cpp/src/arrow/engine/substrait/expression_internal.cc
+++ b/cpp/src/arrow/engine/substrait/expression_internal.cc
@@ -336,8 +336,9 @@ Result<compute::Expression> FromProto(const 
substrait::Expression& expr,
       if (cast_exp.failure_behavior() ==
           substrait::Expression::Cast::FailureBehavior::
               
Expression_Cast_FailureBehavior_FAILURE_BEHAVIOR_THROW_EXCEPTION) {
-        return compute::call("cast", {std::move(input)},
-                             
compute::CastOptions::Safe(std::move(type_nullable.first)));
+        return compute::call(
+            "cast", {std::move(input)},
+            compute::CastOptions::Unsafe(std::move(type_nullable.first)));
       } else if (cast_exp.failure_behavior() ==
                  substrait::Expression::Cast::FailureBehavior::
                      
Expression_Cast_FailureBehavior_FAILURE_BEHAVIOR_RETURN_NULL) {
diff --git a/cpp/src/arrow/engine/substrait/serde_test.cc 
b/cpp/src/arrow/engine/substrait/serde_test.cc
index 06c3cc2096..32b3b5c5c1 100644
--- a/cpp/src/arrow/engine/substrait/serde_test.cc
+++ b/cpp/src/arrow/engine/substrait/serde_test.cc
@@ -814,6 +814,19 @@ TEST(Substrait, Cast) {
   ASSERT_TRUE(expr.call());
 
   ASSERT_THAT(expr.call()->arguments[0].call()->function_name, "cast");
+
+  std::shared_ptr<compute::FunctionOptions> call_opts =
+      expr.call()->arguments[0].call()->options;
+
+  ASSERT_TRUE(!!call_opts);
+  std::shared_ptr<compute::CastOptions> cast_opts =
+      std::dynamic_pointer_cast<compute::CastOptions>(call_opts);
+  ASSERT_TRUE(!!cast_opts);
+  // It is unclear whether a Substrait cast should be safe or not.  In the 
meantime we are
+  // assuming it is unsafe based on the behavior of many SQL engines.
+  ASSERT_TRUE(cast_opts->allow_int_overflow);
+  ASSERT_TRUE(cast_opts->allow_float_truncate);
+  ASSERT_TRUE(cast_opts->allow_decimal_truncate);
 }
 
 TEST(Substrait, CastRequiresFailureBehavior) {