Skip to content

Commit

Permalink
Fixed copy semantic for non composed layouts (#243)
Browse files Browse the repository at this point in the history
  • Loading branch information
JohanMabille authored Oct 18, 2024
1 parent 2781ccc commit 68335e5
Show file tree
Hide file tree
Showing 6 changed files with 157 additions and 1 deletion.
56 changes: 56 additions & 0 deletions include/sparrow/layout/dictionary_encoded_array.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,12 @@ namespace sparrow

explicit dictionary_encoded_array(arrow_proxy);

dictionary_encoded_array(const self_type&);
self_type& operator=(const self_type&);

dictionary_encoded_array(self_type&&);
self_type& operator=(self_type&&);

size_type size() const;

const_reference operator[](size_type i) const;
Expand Down Expand Up @@ -139,6 +145,9 @@ namespace sparrow
friend class array_wrapper_impl;
};

template <class IT>
bool operator==(const dictionary_encoded_array<IT>& lhs, const dictionary_encoded_array<IT>& rhs);

/*******************************************
* dictionary_encoded_array implementation *
*******************************************/
Expand All @@ -152,6 +161,47 @@ namespace sparrow
SPARROW_ASSERT_TRUE(data_type_is_integer(m_proxy.data_type()));
}

template <std::integral IT>
dictionary_encoded_array<IT>::dictionary_encoded_array(const self_type& rhs)
: m_proxy(rhs.m_proxy)
, m_keys_layout(create_keys_layout(m_proxy))
, p_values_layout(create_values_layout(m_proxy))
{
}

template <std::integral IT>
auto dictionary_encoded_array<IT>::operator=(const self_type& rhs) -> self_type&
{
if (this != &rhs)
{
m_proxy = rhs.m_proxy;
m_keys_layout = create_keys_layout(m_proxy);
p_values_layout = create_values_layout(m_proxy);
}
return *this;
}

template <std::integral IT>
dictionary_encoded_array<IT>::dictionary_encoded_array(self_type&& rhs)
: m_proxy(std::move(rhs.m_proxy))
, m_keys_layout(create_keys_layout(m_proxy))
, p_values_layout(create_values_layout(m_proxy))
{
}

template <std::integral IT>
auto dictionary_encoded_array<IT>::operator=(self_type&& rhs) -> self_type&
{
if (this != &rhs)
{
using std::swap;
swap(m_proxy, rhs.m_proxy);
m_keys_layout = create_keys_layout(m_proxy);
p_values_layout = create_values_layout(m_proxy);
}
return *this;
}

template <std::integral IT>
auto dictionary_encoded_array<IT>::size() const -> size_type
{
Expand Down Expand Up @@ -257,4 +307,10 @@ namespace sparrow
{
return m_proxy;
}

template <class IT>
bool operator==(const dictionary_encoded_array<IT>& lhs, const dictionary_encoded_array<IT>& rhs)
{
return std::ranges::equal(lhs, rhs);
}
}
2 changes: 1 addition & 1 deletion src/arrow_array_schema_proxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ namespace sparrow
{
other.m_array = {};
other.m_schema = {};
other.m_buffers.clear();
other.reset();
update_buffers();
update_children();
update_dictionary();
Expand Down
23 changes: 23 additions & 0 deletions test/test_dictionary_encoded_array.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,29 @@ namespace sparrow
CHECK_NOTHROW(layout_type{make_arrow_proxy()});
}

TEST_CASE("copy")
{
layout_type ar(make_arrow_proxy());
layout_type ar2(ar);
CHECK_EQ(ar, ar2);

layout_type ar3(make_arrow_proxy());
ar3 = ar;
CHECK_EQ(ar, ar3);
}

TEST_CASE("move")
{
layout_type ar(make_arrow_proxy());
layout_type ar2(ar);
layout_type ar3(std::move(ar));
CHECK_EQ(ar2, ar3);

layout_type ar4(make_arrow_proxy());
ar4 = std::move(ar3);
CHECK_EQ(ar2, ar4);
}

TEST_CASE("size")
{
const layout_type dict(make_arrow_proxy());
Expand Down
27 changes: 27 additions & 0 deletions test/test_null_array.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,33 @@ namespace sparrow
CHECK_EQ(ar.size(), size);
}

TEST_CASE("copy")
{
constexpr std::size_t size = 10u;
null_array ar(make_arrow_proxy<null_type>(size));
null_array ar2(ar);
CHECK_EQ(ar, ar2);

null_array ar3(make_arrow_proxy<null_type>(size + 2u));
CHECK_NE(ar, ar3);
ar3 = ar;
CHECK_EQ(ar, ar3);
}

TEST_CASE("move")
{
constexpr std::size_t size = 10u;
null_array ar(make_arrow_proxy<null_type>(size));
null_array ar2(ar);
null_array ar3(std::move(ar));
CHECK_EQ(ar3, ar2);

null_array ar4(make_arrow_proxy<null_type>(size + 3u));
CHECK_NE(ar4, ar2);
ar4 = std::move(ar3);
CHECK_EQ(ar2, ar4);
}

TEST_CASE("operator[]")
{
constexpr std::size_t size = 10u;
Expand Down
27 changes: 27 additions & 0 deletions test/test_primitive_array.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,33 @@ namespace sparrow
CHECK_EQ(ar.size(), size - offset);
}

TEST_CASE("copy")
{
array_test_type ar(make_arrow_proxy<scalar_value_type>(size, offset));
array_test_type ar2(ar);

CHECK_EQ(ar, ar2);

array_test_type ar3(make_arrow_proxy<scalar_value_type>(size + 3u, offset));
CHECK_NE(ar, ar3);
ar3 = ar;
CHECK_EQ(ar, ar3);
}

TEST_CASE("move")
{
array_test_type ar(make_arrow_proxy<scalar_value_type>(size, offset));
array_test_type ar2(ar);

array_test_type ar3(std::move(ar));
CHECK_EQ(ar2, ar3);

array_test_type ar4(make_arrow_proxy<scalar_value_type>(size + 3u, offset));
CHECK_NE(ar2, ar4);
ar4 = std::move(ar2);
CHECK_EQ(ar3, ar4);
}

TEST_CASE("const operator[]")
{
auto pr = make_arrow_proxy<scalar_value_type>(size, offset);
Expand Down
23 changes: 23 additions & 0 deletions test/test_variable_size_binary_array.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,29 @@ namespace sparrow
}
}

TEST_CASE_FIXTURE(variable_size_binary_fixture, "copy")
{
layout_type ar(m_arrow_proxy);
layout_type ar2(ar);
CHECK_EQ(ar, ar2);

layout_type ar3(std::move(m_arrow_proxy));
ar3 = ar2;
CHECK_EQ(ar2, ar3);
}

TEST_CASE_FIXTURE(variable_size_binary_fixture, "move")
{
layout_type ar(m_arrow_proxy);
layout_type ar2(ar);
layout_type ar3(std::move(ar));
CHECK_EQ(ar2, ar3);

layout_type ar4(std::move(m_arrow_proxy));
ar4 = std::move(ar3);
CHECK_EQ(ar2, ar4);
}

TEST_CASE_FIXTURE(variable_size_binary_fixture, "size")
{
const layout_type array(std::move(m_arrow_proxy));
Expand Down

0 comments on commit 68335e5

Please sign in to comment.