From 87b149aa1bdcab05f93922c869278845412855dd Mon Sep 17 00:00:00 2001 From: mwish Date: Wed, 29 Jan 2025 01:22:08 +0800 Subject: [PATCH 1/4] GH-45362: Fix Identity cast for timestamp --- cpp/src/arrow/scalar.cc | 2 +- cpp/src/arrow/scalar_test.cc | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/scalar.cc b/cpp/src/arrow/scalar.cc index 85ceec97202..21dbda3b96f 100644 --- a/cpp/src/arrow/scalar.cc +++ b/cpp/src/arrow/scalar.cc @@ -1177,7 +1177,7 @@ enable_if_duration>> CastImpl( } // time to time -template +template enable_if_time>> CastImpl( const TimeScalar& from, std::shared_ptr to_type) { using ToScalar = typename TypeTraits::ScalarType; diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc index d19d7f8a39e..cbed7f1470a 100644 --- a/cpp/src/arrow/scalar_test.cc +++ b/cpp/src/arrow/scalar_test.cc @@ -866,6 +866,9 @@ TEST(TestTimeScalars, Basics) { ASSERT_TRUE(first->Equals(*MakeScalar(ty, 5).ValueOrDie())); ASSERT_TRUE(last->Equals(*MakeScalar(ty, 42).ValueOrDie())); ASSERT_FALSE(last->Equals(*MakeScalar("string"))); + + ASSERT_OK_AND_ASSIGN(auto casted, first->CastTo(ty)); + ASSERT_TRUE(casted->Equals(*first)); } } From cbe36278e0982287359d27b161d16371b8779e84 Mon Sep 17 00:00:00 2001 From: mwish Date: Wed, 29 Jan 2025 18:04:18 +0800 Subject: [PATCH 2/4] Fix compile and use TypeClass of Scalar --- cpp/src/arrow/scalar.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cpp/src/arrow/scalar.cc b/cpp/src/arrow/scalar.cc index 21dbda3b96f..63affc80a06 100644 --- a/cpp/src/arrow/scalar.cc +++ b/cpp/src/arrow/scalar.cc @@ -1177,14 +1177,16 @@ enable_if_duration>> CastImpl( } // time to time -template +template ::ScalarType::TypeClass> enable_if_time>> CastImpl( const TimeScalar& from, std::shared_ptr to_type) { using ToScalar = typename TypeTraits::ScalarType; ARROW_ASSIGN_OR_RAISE( auto value, util::ConvertTimestampValue(AsTimestampType(from.type), AsTimestampType(to_type), from.value)); - return std::make_shared(value, std::move(to_type)); + return std::make_shared(static_cast(value), + std::move(to_type)); } constexpr int64_t kMillisecondsInDay = 86400000; From 6495e3eefa41e6f52e53a4ef66a4157e1925593c Mon Sep 17 00:00:00 2001 From: mwish Date: Wed, 5 Feb 2025 13:40:30 +0800 Subject: [PATCH 3/4] Add testing and also fix the list type castTo --- cpp/src/arrow/scalar.cc | 5 +++-- cpp/src/arrow/scalar_test.cc | 40 ++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/cpp/src/arrow/scalar.cc b/cpp/src/arrow/scalar.cc index 63affc80a06..f9a56c79c9d 100644 --- a/cpp/src/arrow/scalar.cc +++ b/cpp/src/arrow/scalar.cc @@ -1290,10 +1290,11 @@ CastImpl(const StructScalar& from, std::shared_ptr to_type) { } // casts between variable-length and fixed-length list types -template +template std::enable_if_t::value && is_list_type::value, Result>> -CastImpl(const From& from, std::shared_ptr to_type) { +CastImpl(const FromScalar& from, std::shared_ptr to_type) { if constexpr (sizeof(typename To::offset_type) < sizeof(int64_t)) { if (from.value->length() > std::numeric_limits::max()) { return Status::Invalid(from.type->ToString(), " too large to cast to ", diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc index cbed7f1470a..364a90916cd 100644 --- a/cpp/src/arrow/scalar_test.cc +++ b/cpp/src/arrow/scalar_test.cc @@ -36,6 +36,7 @@ #include "arrow/status.h" #include "arrow/testing/extension_type.h" #include "arrow/testing/gtest_util.h" +#include "arrow/testing/random.h" #include "arrow/testing/util.h" #include "arrow/type_traits.h" @@ -156,6 +157,45 @@ TEST(TestBooleanScalar, Cast) { } } +TEST(TestScalar, IndentityCast) { + random::RandomArrayGenerator gen(/*seed=*/42); + auto test_identity_cast_for_type = + [&gen](const std::shared_ptr& data_type) { + auto tmp_array = gen.ArrayOf(data_type, /*size=*/1, /*null_probability=*/0.0); + ARROW_SCOPED_TRACE("data type = ", data_type->ToString()); + ASSERT_OK_AND_ASSIGN(auto scalar, tmp_array->GetScalar(0)); + ASSERT_OK_AND_ASSIGN(auto casted_scalar, scalar->CastTo(data_type)); + ASSERT_TRUE(casted_scalar->Equals(*scalar)); + ASSERT_TRUE(scalar->Equals(*casted_scalar)); + }; + for (auto& type : PrimitiveTypes()) { + test_identity_cast_for_type(type); + } + for (auto& type : DurationTypes()) { + test_identity_cast_for_type(type); + } + for (auto& type : IntervalTypes()) { + test_identity_cast_for_type(type); + } + for (auto& type : { + arrow::fixed_size_list(arrow::int32(), 20), arrow::list(arrow::int32()), + // arrow::map(arrow::binary(), arrow::int32()), + // struct_({field("float", arrow::float32())}), + }) { + test_identity_cast_for_type(type); + } + /* + for (auto& type: { + arrow::decimal32(2, 2), + arrow::decimal64(4, 4), + arrow::decimal128(10, 10), + arrow::decimal128(20, 20), + }) { + test_identity_cast_for_type(type); + } + */ +} + template class TestNumericScalar : public ::testing::Test { public: From 4bed2d707bdb997c1402017d88202abadc2d5fb3 Mon Sep 17 00:00:00 2001 From: mwish Date: Wed, 5 Feb 2025 21:19:25 +0800 Subject: [PATCH 4/4] Address comments --- cpp/src/arrow/scalar_test.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc index 364a90916cd..6938bc0d887 100644 --- a/cpp/src/arrow/scalar_test.cc +++ b/cpp/src/arrow/scalar_test.cc @@ -157,7 +157,7 @@ TEST(TestBooleanScalar, Cast) { } } -TEST(TestScalar, IndentityCast) { +TEST(TestScalar, IdentityCast) { random::RandomArrayGenerator gen(/*seed=*/42); auto test_identity_cast_for_type = [&gen](const std::shared_ptr& data_type) { @@ -179,11 +179,16 @@ TEST(TestScalar, IndentityCast) { } for (auto& type : { arrow::fixed_size_list(arrow::int32(), 20), arrow::list(arrow::int32()), + arrow::large_list(arrow::int32()), + // TODO(GH-45430): CastTo for ListView is not implemented yet. + // arrow::list_view(arrow::int32()), arrow::large_list_view(arrow::int32()) + // TODO(GH-45431): CastTo for ComplexType is not implemented yet. // arrow::map(arrow::binary(), arrow::int32()), // struct_({field("float", arrow::float32())}), }) { test_identity_cast_for_type(type); } + // TODO(GH-45429): CastTo for Decimal is not implemented yet. /* for (auto& type: { arrow::decimal32(2, 2),