From 4c08fc30aba45d5499fe58829cb1ea048b37981f Mon Sep 17 00:00:00 2001 From: Hyunseok Seo Date: Tue, 5 Dec 2023 00:43:22 +0900 Subject: [PATCH 01/16] Add casting from (large)listview to (large)list --- cpp/src/arrow/compute/kernels/scalar_cast_nested.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc index 6fd449a9313..637f76c7874 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc @@ -480,14 +480,18 @@ std::vector> GetNestedCasts() { auto cast_list = std::make_shared("cast_list", Type::LIST); AddCommonCasts(Type::LIST, kOutputTargetType, cast_list.get()); AddListCast(cast_list.get()); + AddListCast(cast_list.get()); AddListCast(cast_list.get()); + AddListCast(cast_list.get()); AddTypeToTypeCast, FixedSizeListType>(cast_list.get()); auto cast_large_list = std::make_shared("cast_large_list", Type::LARGE_LIST); AddCommonCasts(Type::LARGE_LIST, kOutputTargetType, cast_large_list.get()); AddListCast(cast_large_list.get()); + AddListCast(cast_large_list.get()); AddListCast(cast_large_list.get()); + AddListCast(cast_large_list.get()); AddTypeToTypeCast, FixedSizeListType>( cast_large_list.get()); From 8317b4c3538e6d21ca21b1e8662a3fab8c527a14 Mon Sep 17 00:00:00 2001 From: Hyunseok Seo Date: Fri, 22 Dec 2023 18:41:16 +0900 Subject: [PATCH 02/16] Add casting from (large)list view to fixed size list --- cpp/src/arrow/compute/kernels/scalar_cast_nested.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc index 637f76c7874..8fc6952929c 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc @@ -401,7 +401,7 @@ void AddTypeToTypeCast(CastFunction* func) { kernel.exec = CastFunctor::Exec; kernel.signature = KernelSignature::Make({InputType(SrcT::type_id)}, kOutputTargetType); kernel.null_handling = NullHandling::COMPUTED_NO_PREALLOCATE; - DCHECK_OK(func->AddKernel(StructType::type_id, std::move(kernel))); + DCHECK_OK(func->AddKernel(SrcT::type_id, std::move(kernel))); } template @@ -507,7 +507,10 @@ std::vector> GetNestedCasts() { AddCommonCasts(Type::FIXED_SIZE_LIST, kOutputTargetType, cast_fsl.get()); AddTypeToTypeCast(cast_fsl.get()); AddTypeToTypeCast, ListType>(cast_fsl.get()); + AddTypeToTypeCast, ListViewType>(cast_fsl.get()); AddTypeToTypeCast, LargeListType>(cast_fsl.get()); + AddTypeToTypeCast, LargeListViewType>( + cast_fsl.get()); // So is struct auto cast_struct = std::make_shared("cast_struct", Type::STRUCT); From 76f6b4911ff20470a1f356c2d888798ac213496a Mon Sep 17 00:00:00 2001 From: Hyunseok Seo Date: Fri, 22 Dec 2023 18:52:20 +0900 Subject: [PATCH 03/16] Add casting from map to fixed size list --- cpp/src/arrow/compute/kernels/scalar_cast_nested.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc index 8fc6952929c..e0b5ce612e4 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc @@ -511,6 +511,7 @@ std::vector> GetNestedCasts() { AddTypeToTypeCast, LargeListType>(cast_fsl.get()); AddTypeToTypeCast, LargeListViewType>( cast_fsl.get()); + AddMapCast(cast_fsl.get()); // So is struct auto cast_struct = std::make_shared("cast_struct", Type::STRUCT); From ef385526cfd76cada4c5eb64bb5b4965e5f659a4 Mon Sep 17 00:00:00 2001 From: Hyunseok Seo Date: Fri, 22 Dec 2023 22:03:01 +0900 Subject: [PATCH 04/16] Fix CheckInvalidListCast for TestListLikeScalar --- cpp/src/arrow/scalar_test.cc | 51 ++++++++++++++++++++++++------------ 1 file changed, 34 insertions(+), 17 deletions(-) diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc index 9d40e688f1d..43815ea60fc 100644 --- a/cpp/src/arrow/scalar_test.cc +++ b/cpp/src/arrow/scalar_test.cc @@ -1077,21 +1077,38 @@ std::shared_ptr MakeListType( template void CheckListCast(const ScalarType& scalar, const std::shared_ptr& to_type) { - EXPECT_OK_AND_ASSIGN(auto cast_scalar, scalar.CastTo(to_type)); - ASSERT_OK(cast_scalar->ValidateFull()); - ASSERT_EQ(*cast_scalar->type, *to_type); + EXPECT_OK_AND_ASSIGN(auto cast_scalar, Cast(scalar, to_type)); + ASSERT_OK(cast_scalar.scalar()->ValidateFull()); + ASSERT_EQ(*cast_scalar.scalar()->type, *to_type); - ASSERT_EQ(scalar.is_valid, cast_scalar->is_valid); + ASSERT_EQ(scalar.is_valid, cast_scalar.scalar()->is_valid); ASSERT_TRUE(scalar.is_valid); ASSERT_ARRAYS_EQUAL(*scalar.value, - *checked_cast(*cast_scalar).value); + *checked_cast(*cast_scalar.scalar()).value); } -void CheckInvalidListCast(const Scalar& scalar, const std::shared_ptr& to_type, +std::tuple GetExpectedError( + const std::shared_ptr& type, + const std::shared_ptr& invalidCastType) { + if (type->id() == Type::FIXED_SIZE_LIST) { + return std::make_tuple( + StatusCode::TypeError, + "Size of FixedSizeList is not the same. input list: " + type->ToString() + + " output list: " + invalidCastType->ToString()); + } else { + return std::make_tuple( + StatusCode::Invalid, + "ListType can only be casted to FixedSizeListType if the lists are all the " + "expected size."); + } +} + +template +void CheckInvalidListCast(const ScalarType& scalar, + const std::shared_ptr& to_type, const StatusCode code, const std::string& expected_message) { - EXPECT_RAISES_WITH_CODE_AND_MESSAGE_THAT(StatusCode::Invalid, - ::testing::HasSubstr(expected_message), - scalar.CastTo(to_type)); + EXPECT_RAISES_WITH_CODE_AND_MESSAGE_THAT(code, ::testing::HasSubstr(expected_message), + Cast(scalar, to_type)); } template @@ -1178,10 +1195,10 @@ class TestListLikeScalar : public ::testing::Test { CheckListCast( scalar, fixed_size_list(value_->type(), static_cast(value_->length()))); - CheckInvalidListCast(scalar, fixed_size_list(value_->type(), 5), - "Cannot cast " + scalar.type->ToString() + " of length " + - std::to_string(value_->length()) + - " to fixed size list of length 5"); + auto invalidCastType = fixed_size_list(value_->type(), 5); + auto [expectedCode, expectedMessage] = GetExpectedError(type_, invalidCastType); + + CheckInvalidListCast(scalar, invalidCastType, expectedCode, expectedMessage); } protected: @@ -1238,10 +1255,10 @@ TEST(TestMapScalar, Cast) { CheckListCast(scalar, large_list(key_value_type)); CheckListCast(scalar, fixed_size_list(key_value_type, 2)); - CheckInvalidListCast(scalar, fixed_size_list(key_value_type, 5), - "Cannot cast " + scalar.type->ToString() + " of length " + - std::to_string(value->length()) + - " to fixed size list of length 5"); +// CheckInvalidListCast(scalar, fixed_size_list(key_value_type, 5), +// "Cannot cast " + scalar.type->ToString() + " of length " + +// std::to_string(value->length()) + +// " to fixed size list of length 5"); } TEST(TestStructScalar, FieldAccess) { From bb2d07aeee4960faa7e2e4ebd6700e88cd05161b Mon Sep 17 00:00:00 2001 From: Hyunseok Seo Date: Fri, 22 Dec 2023 22:39:02 +0900 Subject: [PATCH 05/16] Apply Lint --- cpp/src/arrow/scalar_test.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc index 43815ea60fc..3f4cade680a 100644 --- a/cpp/src/arrow/scalar_test.cc +++ b/cpp/src/arrow/scalar_test.cc @@ -1255,10 +1255,10 @@ TEST(TestMapScalar, Cast) { CheckListCast(scalar, large_list(key_value_type)); CheckListCast(scalar, fixed_size_list(key_value_type, 2)); -// CheckInvalidListCast(scalar, fixed_size_list(key_value_type, 5), -// "Cannot cast " + scalar.type->ToString() + " of length " + -// std::to_string(value->length()) + -// " to fixed size list of length 5"); + // CheckInvalidListCast(scalar, fixed_size_list(key_value_type, 5), + // "Cannot cast " + scalar.type->ToString() + " of length " + + // std::to_string(value->length()) + + // " to fixed size list of length 5"); } TEST(TestStructScalar, FieldAccess) { From 39e88daa9ae5fc2b7bc43dd595eb5d8e35e63b67 Mon Sep 17 00:00:00 2001 From: Hyunseok Seo Date: Sat, 23 Dec 2023 17:05:27 +0900 Subject: [PATCH 06/16] Add validation for CastMap --- .../arrow/compute/kernels/scalar_cast_nested.cc | 17 +++++++++++++++-- cpp/src/arrow/scalar_test.cc | 14 +++++++------- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc index e0b5ce612e4..c74c64460b2 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc @@ -317,8 +317,8 @@ struct CastFixedList { if (in_size != out_size) { return Status::TypeError("Size of FixedSizeList is not the same.", - " input list: ", in_type.ToString(), - " output list: ", out_type.ToString()); + " input type: ", in_type.ToString(), + " output type: ", out_type.ToString()); } const ArraySpan& in_array = batch[0].array; @@ -429,6 +429,19 @@ struct CastMap { std::shared_ptr entries = in_array.child_data[0].ToArrayData(); + // Validate if output is fixed size list + if (out->type()->id() == Type::FIXED_SIZE_LIST) { + const auto& in_type = checked_cast(*batch[0].type()); + const auto& out_type = checked_cast(*out->type()); + const auto in_size = entries->length; + const auto out_size = out_type.list_size(); + if (in_size != out_size) { + return Status::TypeError("Size of FixedSizeList is not the same.", + " input type: ", in_type.ToString(), + " output type: ", out_type.ToString()); + } + } + // Shift bitmap in case the source offset is non-zero if (in_array.offset != 0 && in_array.buffers[0].data != nullptr) { ARROW_ASSIGN_OR_RAISE(out_array->buffers[0], diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc index 3f4cade680a..16e7eb81779 100644 --- a/cpp/src/arrow/scalar_test.cc +++ b/cpp/src/arrow/scalar_test.cc @@ -1090,11 +1090,11 @@ void CheckListCast(const ScalarType& scalar, const std::shared_ptr& to std::tuple GetExpectedError( const std::shared_ptr& type, const std::shared_ptr& invalidCastType) { - if (type->id() == Type::FIXED_SIZE_LIST) { + if (type->id() == Type::FIXED_SIZE_LIST || type->id() == Type::MAP) { return std::make_tuple( StatusCode::TypeError, - "Size of FixedSizeList is not the same. input list: " + type->ToString() + - " output list: " + invalidCastType->ToString()); + "Size of FixedSizeList is not the same. input type: " + type->ToString() + + " output type: " + invalidCastType->ToString()); } else { return std::make_tuple( StatusCode::Invalid, @@ -1255,10 +1255,10 @@ TEST(TestMapScalar, Cast) { CheckListCast(scalar, large_list(key_value_type)); CheckListCast(scalar, fixed_size_list(key_value_type, 2)); - // CheckInvalidListCast(scalar, fixed_size_list(key_value_type, 5), - // "Cannot cast " + scalar.type->ToString() + " of length " + - // std::to_string(value->length()) + - // " to fixed size list of length 5"); + auto invalidCastType = fixed_size_list(key_value_type, 5); + auto [expectedCode, expectedMessage] = GetExpectedError(scalar.type, invalidCastType); + + CheckInvalidListCast(scalar, invalidCastType, expectedCode, expectedMessage); } TEST(TestStructScalar, FieldAccess) { From 2924125036465f61d3cd3ad2c111fc4b68f09d02 Mon Sep 17 00:00:00 2001 From: Hyunseok Seo Date: Sat, 23 Dec 2023 17:45:04 +0900 Subject: [PATCH 07/16] Move validation logic to avoid segmentation fault --- .../compute/kernels/scalar_cast_nested.cc | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc index c74c64460b2..873d250b2d4 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc @@ -410,6 +410,18 @@ struct CastMap { static Status Exec(KernelContext* ctx, const ExecSpan& batch, ExecResult* out) { const CastOptions& options = CastState::Get(ctx); + // Validate if output is fixed size list + if (out->type()->id() == Type::FIXED_SIZE_LIST) { + const auto& in_type = checked_cast(*batch[0].type()); + const auto& out_type = checked_cast(*out->type()); + const auto in_size = batch[0].array.child_data[0].ToArrayData()->length; + const auto out_size = out_type.list_size(); + if (in_size != out_size) { + return Status::TypeError("Size of FixedSizeList is not the same.", + " input type: ", in_type.ToString(), + " output type: ", out_type.ToString()); + } + } std::shared_ptr entry_type = checked_cast(*out->type()).value_type(); @@ -429,19 +441,6 @@ struct CastMap { std::shared_ptr entries = in_array.child_data[0].ToArrayData(); - // Validate if output is fixed size list - if (out->type()->id() == Type::FIXED_SIZE_LIST) { - const auto& in_type = checked_cast(*batch[0].type()); - const auto& out_type = checked_cast(*out->type()); - const auto in_size = entries->length; - const auto out_size = out_type.list_size(); - if (in_size != out_size) { - return Status::TypeError("Size of FixedSizeList is not the same.", - " input type: ", in_type.ToString(), - " output type: ", out_type.ToString()); - } - } - // Shift bitmap in case the source offset is non-zero if (in_array.offset != 0 && in_array.buffers[0].data != nullptr) { ARROW_ASSIGN_OR_RAISE(out_array->buffers[0], From 7ca7195f253aefd6f4e4d1a4f0e0eca3c674edac Mon Sep 17 00:00:00 2001 From: Hyunseok Seo Date: Sat, 23 Dec 2023 19:20:46 +0900 Subject: [PATCH 08/16] Assign null bitmap buffer using GetNullBitmapBuffer function --- cpp/src/arrow/compute/kernels/scalar_cast_nested.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc index 873d250b2d4..f18a0f5f5df 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc @@ -436,7 +436,8 @@ struct CastMap { const ArraySpan& in_array = batch[0].array; ArrayData* out_array = out->array_data().get(); - out_array->buffers[0] = in_array.GetBuffer(0); + ARROW_ASSIGN_OR_RAISE(out_array->buffers[0], + GetNullBitmapBuffer(in_array, ctx->memory_pool())); out_array->buffers[1] = in_array.GetBuffer(1); std::shared_ptr entries = in_array.child_data[0].ToArrayData(); From 3c455ccc3917aaf807572598eb7ad2f16fa7d796 Mon Sep 17 00:00:00 2001 From: Hyunseok Seo Date: Sat, 23 Dec 2023 19:50:26 +0900 Subject: [PATCH 09/16] Add null check --- cpp/src/arrow/compute/kernels/scalar_cast_nested.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc index f18a0f5f5df..8f3c5f8a6c9 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc @@ -438,7 +438,9 @@ struct CastMap { ArrayData* out_array = out->array_data().get(); ARROW_ASSIGN_OR_RAISE(out_array->buffers[0], GetNullBitmapBuffer(in_array, ctx->memory_pool())); - out_array->buffers[1] = in_array.GetBuffer(1); + if (in_array.GetBuffer(1) != nullptr) { + out_array->buffers[1] = in_array.GetBuffer(1); + } std::shared_ptr entries = in_array.child_data[0].ToArrayData(); From 886b76e2a208385ffe86ef5ad6642ad49c494641 Mon Sep 17 00:00:00 2001 From: Hyunseok Seo Date: Sat, 23 Dec 2023 20:29:47 +0900 Subject: [PATCH 10/16] Fix casting from map to fixed size list --- .../compute/kernels/scalar_cast_nested.cc | 21 +++---------------- cpp/src/arrow/scalar_test.cc | 2 +- 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc index 8f3c5f8a6c9..c083171243e 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc @@ -410,18 +410,6 @@ struct CastMap { static Status Exec(KernelContext* ctx, const ExecSpan& batch, ExecResult* out) { const CastOptions& options = CastState::Get(ctx); - // Validate if output is fixed size list - if (out->type()->id() == Type::FIXED_SIZE_LIST) { - const auto& in_type = checked_cast(*batch[0].type()); - const auto& out_type = checked_cast(*out->type()); - const auto in_size = batch[0].array.child_data[0].ToArrayData()->length; - const auto out_size = out_type.list_size(); - if (in_size != out_size) { - return Status::TypeError("Size of FixedSizeList is not the same.", - " input type: ", in_type.ToString(), - " output type: ", out_type.ToString()); - } - } std::shared_ptr entry_type = checked_cast(*out->type()).value_type(); @@ -436,11 +424,8 @@ struct CastMap { const ArraySpan& in_array = batch[0].array; ArrayData* out_array = out->array_data().get(); - ARROW_ASSIGN_OR_RAISE(out_array->buffers[0], - GetNullBitmapBuffer(in_array, ctx->memory_pool())); - if (in_array.GetBuffer(1) != nullptr) { - out_array->buffers[1] = in_array.GetBuffer(1); - } + out_array->buffers[0] = in_array.GetBuffer(0); + out_array->buffers[1] = in_array.GetBuffer(1); std::shared_ptr entries = in_array.child_data[0].ToArrayData(); @@ -526,7 +511,7 @@ std::vector> GetNestedCasts() { AddTypeToTypeCast, LargeListType>(cast_fsl.get()); AddTypeToTypeCast, LargeListViewType>( cast_fsl.get()); - AddMapCast(cast_fsl.get()); + AddTypeToTypeCast, MapType>(cast_fsl.get()); // So is struct auto cast_struct = std::make_shared("cast_struct", Type::STRUCT); diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc index 16e7eb81779..a26c07091a9 100644 --- a/cpp/src/arrow/scalar_test.cc +++ b/cpp/src/arrow/scalar_test.cc @@ -1090,7 +1090,7 @@ void CheckListCast(const ScalarType& scalar, const std::shared_ptr& to std::tuple GetExpectedError( const std::shared_ptr& type, const std::shared_ptr& invalidCastType) { - if (type->id() == Type::FIXED_SIZE_LIST || type->id() == Type::MAP) { + if (type->id() == Type::FIXED_SIZE_LIST) { return std::make_tuple( StatusCode::TypeError, "Size of FixedSizeList is not the same. input type: " + type->ToString() + From 44cd40457aeeda00a03721b5100e4852419afd3a Mon Sep 17 00:00:00 2001 From: Hyunseok Seo Date: Sat, 23 Dec 2023 23:34:01 +0900 Subject: [PATCH 11/16] Rollback message --- cpp/src/arrow/compute/kernels/scalar_cast_nested.cc | 4 ++-- cpp/src/arrow/scalar_test.cc | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc index c083171243e..ec5291ef608 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc @@ -317,8 +317,8 @@ struct CastFixedList { if (in_size != out_size) { return Status::TypeError("Size of FixedSizeList is not the same.", - " input type: ", in_type.ToString(), - " output type: ", out_type.ToString()); + " input list: ", in_type.ToString(), + " output list: ", out_type.ToString()); } const ArraySpan& in_array = batch[0].array; diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc index a26c07091a9..57fabd8e30e 100644 --- a/cpp/src/arrow/scalar_test.cc +++ b/cpp/src/arrow/scalar_test.cc @@ -1093,8 +1093,8 @@ std::tuple GetExpectedError( if (type->id() == Type::FIXED_SIZE_LIST) { return std::make_tuple( StatusCode::TypeError, - "Size of FixedSizeList is not the same. input type: " + type->ToString() + - " output type: " + invalidCastType->ToString()); + "Size of FixedSizeList is not the same. input list: " + type->ToString() + + " output list: " + invalidCastType->ToString()); } else { return std::make_tuple( StatusCode::Invalid, From d8771a1ae0b7d383de10c520dded31984fd71e86 Mon Sep 17 00:00:00 2001 From: Hyunseok Seo Date: Tue, 26 Dec 2023 11:37:59 +0900 Subject: [PATCH 12/16] Change variable naming --- cpp/src/arrow/scalar_test.cc | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc index 57fabd8e30e..f4954051f15 100644 --- a/cpp/src/arrow/scalar_test.cc +++ b/cpp/src/arrow/scalar_test.cc @@ -1077,14 +1077,15 @@ std::shared_ptr MakeListType( template void CheckListCast(const ScalarType& scalar, const std::shared_ptr& to_type) { - EXPECT_OK_AND_ASSIGN(auto cast_scalar, Cast(scalar, to_type)); - ASSERT_OK(cast_scalar.scalar()->ValidateFull()); - ASSERT_EQ(*cast_scalar.scalar()->type, *to_type); + EXPECT_OK_AND_ASSIGN(auto cast_scalar_datum, Cast(scalar, to_type)); + const auto& cast_scalar = cast_scalar_datum.scalar(); + ASSERT_OK(cast_scalar->ValidateFull()); + ASSERT_EQ(*cast_scalar->type, *to_type); - ASSERT_EQ(scalar.is_valid, cast_scalar.scalar()->is_valid); + ASSERT_EQ(scalar.is_valid, cast_scalar->is_valid); ASSERT_TRUE(scalar.is_valid); ASSERT_ARRAYS_EQUAL(*scalar.value, - *checked_cast(*cast_scalar.scalar()).value); + *checked_cast(*cast_scalar).value); } std::tuple GetExpectedError( From b67e9f413974d8ff30f9d22fc7ea24b438b6c23f Mon Sep 17 00:00:00 2001 From: Hyunseok Seo Date: Tue, 26 Dec 2023 12:17:23 +0900 Subject: [PATCH 13/16] Change function naming --- cpp/src/arrow/scalar_test.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc index f4954051f15..3170a00e34d 100644 --- a/cpp/src/arrow/scalar_test.cc +++ b/cpp/src/arrow/scalar_test.cc @@ -1105,9 +1105,9 @@ std::tuple GetExpectedError( } template -void CheckInvalidListCast(const ScalarType& scalar, - const std::shared_ptr& to_type, const StatusCode code, - const std::string& expected_message) { +void CheckListCastError(const ScalarType& scalar, + const std::shared_ptr& to_type, const StatusCode code, + const std::string& expected_message) { EXPECT_RAISES_WITH_CODE_AND_MESSAGE_THAT(code, ::testing::HasSubstr(expected_message), Cast(scalar, to_type)); } @@ -1199,7 +1199,7 @@ class TestListLikeScalar : public ::testing::Test { auto invalidCastType = fixed_size_list(value_->type(), 5); auto [expectedCode, expectedMessage] = GetExpectedError(type_, invalidCastType); - CheckInvalidListCast(scalar, invalidCastType, expectedCode, expectedMessage); + CheckListCastError(scalar, invalidCastType, expectedCode, expectedMessage); } protected: @@ -1259,7 +1259,7 @@ TEST(TestMapScalar, Cast) { auto invalidCastType = fixed_size_list(key_value_type, 5); auto [expectedCode, expectedMessage] = GetExpectedError(scalar.type, invalidCastType); - CheckInvalidListCast(scalar, invalidCastType, expectedCode, expectedMessage); + CheckListCastError(scalar, invalidCastType, expectedCode, expectedMessage); } TEST(TestStructScalar, FieldAccess) { From 58f4afc42a112378a81a1db8868952e3647a414d Mon Sep 17 00:00:00 2001 From: Hyunseok Seo Date: Sun, 31 Dec 2023 23:59:38 +0900 Subject: [PATCH 14/16] Change naming --- cpp/src/arrow/scalar_test.cc | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc index 3170a00e34d..313e51264c8 100644 --- a/cpp/src/arrow/scalar_test.cc +++ b/cpp/src/arrow/scalar_test.cc @@ -1090,12 +1090,12 @@ void CheckListCast(const ScalarType& scalar, const std::shared_ptr& to std::tuple GetExpectedError( const std::shared_ptr& type, - const std::shared_ptr& invalidCastType) { + const std::shared_ptr& invalid_cast_type) { if (type->id() == Type::FIXED_SIZE_LIST) { return std::make_tuple( StatusCode::TypeError, "Size of FixedSizeList is not the same. input list: " + type->ToString() + - " output list: " + invalidCastType->ToString()); + " output list: " + invalid_cast_type->ToString()); } else { return std::make_tuple( StatusCode::Invalid, @@ -1196,10 +1196,10 @@ class TestListLikeScalar : public ::testing::Test { CheckListCast( scalar, fixed_size_list(value_->type(), static_cast(value_->length()))); - auto invalidCastType = fixed_size_list(value_->type(), 5); - auto [expectedCode, expectedMessage] = GetExpectedError(type_, invalidCastType); + auto invalid_cast_type = fixed_size_list(value_->type(), 5); + auto [expectedCode, expectedMessage] = GetExpectedError(type_, invalid_cast_type); - CheckListCastError(scalar, invalidCastType, expectedCode, expectedMessage); + CheckListCastError(scalar, invalid_cast_type, expectedCode, expectedMessage); } protected: @@ -1256,10 +1256,10 @@ TEST(TestMapScalar, Cast) { CheckListCast(scalar, large_list(key_value_type)); CheckListCast(scalar, fixed_size_list(key_value_type, 2)); - auto invalidCastType = fixed_size_list(key_value_type, 5); - auto [expectedCode, expectedMessage] = GetExpectedError(scalar.type, invalidCastType); + auto invalid_cast_type = fixed_size_list(key_value_type, 5); + auto [expectedCode, expectedMessage] = GetExpectedError(scalar.type, invalid_cast_type); - CheckListCastError(scalar, invalidCastType, expectedCode, expectedMessage); + CheckListCastError(scalar, invalid_cast_type, expectedCode, expectedMessage); } TEST(TestStructScalar, FieldAccess) { From 5d33359e297d9e7c05a39f5adb79772460bbe7c6 Mon Sep 17 00:00:00 2001 From: Hyunseok Seo Date: Mon, 1 Jan 2024 00:39:43 +0900 Subject: [PATCH 15/16] Remove GetExpectedError function --- cpp/src/arrow/scalar_test.cc | 38 +++++++++++++++--------------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc index 313e51264c8..654e0510c6f 100644 --- a/cpp/src/arrow/scalar_test.cc +++ b/cpp/src/arrow/scalar_test.cc @@ -1088,26 +1088,24 @@ void CheckListCast(const ScalarType& scalar, const std::shared_ptr& to *checked_cast(*cast_scalar).value); } -std::tuple GetExpectedError( - const std::shared_ptr& type, - const std::shared_ptr& invalid_cast_type) { - if (type->id() == Type::FIXED_SIZE_LIST) { - return std::make_tuple( - StatusCode::TypeError, - "Size of FixedSizeList is not the same. input list: " + type->ToString() + - " output list: " + invalid_cast_type->ToString()); +template +void CheckListCastError(const ScalarType& scalar, + const std::shared_ptr& from_type, + const std::shared_ptr& to_type) { + StatusCode code; + std::string expected_message; + if (from_type->id() == Type::FIXED_SIZE_LIST) { + code = StatusCode::TypeError; + expected_message = + "Size of FixedSizeList is not the same. input list: " + from_type->ToString() + + " output list: " + to_type->ToString(); } else { - return std::make_tuple( - StatusCode::Invalid, + code = StatusCode::Invalid; + expected_message = "ListType can only be casted to FixedSizeListType if the lists are all the " - "expected size."); + "expected size."; } -} -template -void CheckListCastError(const ScalarType& scalar, - const std::shared_ptr& to_type, const StatusCode code, - const std::string& expected_message) { EXPECT_RAISES_WITH_CODE_AND_MESSAGE_THAT(code, ::testing::HasSubstr(expected_message), Cast(scalar, to_type)); } @@ -1197,9 +1195,7 @@ class TestListLikeScalar : public ::testing::Test { scalar, fixed_size_list(value_->type(), static_cast(value_->length()))); auto invalid_cast_type = fixed_size_list(value_->type(), 5); - auto [expectedCode, expectedMessage] = GetExpectedError(type_, invalid_cast_type); - - CheckListCastError(scalar, invalid_cast_type, expectedCode, expectedMessage); + CheckListCastError(scalar, type_, invalid_cast_type); } protected: @@ -1257,9 +1253,7 @@ TEST(TestMapScalar, Cast) { CheckListCast(scalar, fixed_size_list(key_value_type, 2)); auto invalid_cast_type = fixed_size_list(key_value_type, 5); - auto [expectedCode, expectedMessage] = GetExpectedError(scalar.type, invalid_cast_type); - - CheckListCastError(scalar, invalid_cast_type, expectedCode, expectedMessage); + CheckListCastError(scalar, scalar.type, invalid_cast_type); } TEST(TestStructScalar, FieldAccess) { From b7832abd1cfb09fed66deee4f91fe0f24d1e4508 Mon Sep 17 00:00:00 2001 From: Hyunseok Seo Date: Mon, 1 Jan 2024 20:17:11 +0900 Subject: [PATCH 16/16] Update CheckListCastError function to remove from_type argument --- cpp/src/arrow/scalar_test.cc | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc index 654e0510c6f..8592765adca 100644 --- a/cpp/src/arrow/scalar_test.cc +++ b/cpp/src/arrow/scalar_test.cc @@ -1090,14 +1090,13 @@ void CheckListCast(const ScalarType& scalar, const std::shared_ptr& to template void CheckListCastError(const ScalarType& scalar, - const std::shared_ptr& from_type, const std::shared_ptr& to_type) { StatusCode code; std::string expected_message; - if (from_type->id() == Type::FIXED_SIZE_LIST) { + if (scalar.type->id() == Type::FIXED_SIZE_LIST) { code = StatusCode::TypeError; expected_message = - "Size of FixedSizeList is not the same. input list: " + from_type->ToString() + + "Size of FixedSizeList is not the same. input list: " + scalar.type->ToString() + " output list: " + to_type->ToString(); } else { code = StatusCode::Invalid; @@ -1195,7 +1194,7 @@ class TestListLikeScalar : public ::testing::Test { scalar, fixed_size_list(value_->type(), static_cast(value_->length()))); auto invalid_cast_type = fixed_size_list(value_->type(), 5); - CheckListCastError(scalar, type_, invalid_cast_type); + CheckListCastError(scalar, invalid_cast_type); } protected: @@ -1253,7 +1252,7 @@ TEST(TestMapScalar, Cast) { CheckListCast(scalar, fixed_size_list(key_value_type, 2)); auto invalid_cast_type = fixed_size_list(key_value_type, 5); - CheckListCastError(scalar, scalar.type, invalid_cast_type); + CheckListCastError(scalar, invalid_cast_type); } TEST(TestStructScalar, FieldAccess) {