-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Constructorsv2 #249
base: main
Are you sure you want to change the base?
Constructorsv2 #249
Changes from 23 commits
a1ea4d3
9401544
c64ba0b
37bac80
74fe539
78b5bf7
bb48696
372ac13
7009c14
672eb37
deb2347
9456fee
f3864bb
b0e7dad
ca2229d
135a991
03860a0
e758a64
1e005e3
c00a77c
45cebd6
158db2d
0bbc89c
4cf6663
3934c68
19d48eb
ab34b8d
a11c003
117f536
6d321fd
6be7797
c1f66ca
0b37081
74e2614
e80420c
2b8e589
1bccd1f
fbbf6ff
78f1e6f
8568f47
75ab2c9
e074fcd
6675e29
9b0b140
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
// Copyright 2024 Man Group Operations Limited | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
|
@@ -17,6 +17,7 @@ | |
#include "sparrow/c_interface.hpp" | ||
#include "sparrow/config/config.hpp" | ||
#include "sparrow/layout/array_wrapper.hpp" | ||
#include "sparrow/layout/array_access.hpp" | ||
#include "sparrow/layout/nested_value_types.hpp" | ||
#include "sparrow/types/data_traits.hpp" | ||
|
||
|
@@ -29,12 +30,19 @@ | |
using size_type = std::size_t; | ||
using value_type = array_traits::value_type; | ||
using const_reference = array_traits::const_reference; | ||
|
||
|
||
// array data will be moved into the array object | ||
template<class ARRAY_TYPE> | ||
requires std::is_rvalue_reference_v<ARRAY_TYPE&&> | ||
array(ARRAY_TYPE&& array); | ||
|
||
SPARROW_API array() = default; | ||
|
||
SPARROW_API array(ArrowArray&& array, ArrowSchema&& schema); | ||
SPARROW_API array(ArrowArray&& array, ArrowSchema* schema); | ||
SPARROW_API array(ArrowArray* array, ArrowSchema* schema); | ||
|
||
|
||
|
||
SPARROW_API bool owns_arrow_array() const; | ||
SPARROW_API array& get_arrow_array(ArrowArray*&); | ||
|
@@ -47,9 +55,21 @@ | |
SPARROW_API size_type size() const; | ||
SPARROW_API const_reference operator[](size_type) const; | ||
|
||
SPARROW_API cloning_ptr<array_wrapper> && extract_array_wrapper() &&; | ||
|
||
private: | ||
|
||
cloning_ptr<array_wrapper> p_array = nullptr; | ||
|
||
friend detail::array_access; | ||
}; | ||
|
||
template<class ARRAY_TYPE> | ||
requires std::is_rvalue_reference_v<ARRAY_TYPE&&> | ||
array::array(ARRAY_TYPE&& arr) | ||
: p_array( new array_wrapper_impl<std::remove_reference_t<ARRAY_TYPE>>(std::move(arr))) | ||
{ | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since ARRAY_TYPE is known at compile time, maybe something like: template<class ARRAY_TYPE>
requires std::is_rvalue_reference_v<ARRAY_TYPE&&>
array::array(ARRAY_TYPE&& array)
: p_array(new array_wrapper_impl<ARRAY_TYPE>(std::move(array)))
{
} would be enough? It avoids extracting the proxy and going through the factory. |
||
|
||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
// Copyright 2024 Man Group Operations Limited | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
#pragma once | ||
|
||
#include "sparrow/arrow_array_schema_proxy.hpp" | ||
|
||
|
||
namespace sparrow::detail | ||
{ | ||
|
||
class array_access | ||
{ | ||
public: | ||
template<class ARRAY> | ||
static inline const sparrow::arrow_proxy& storage(const ARRAY& array) | ||
{ | ||
return array.storage(); | ||
} | ||
|
||
template<class ARRAY> | ||
static inline sparrow::arrow_proxy& storage(ARRAY& array) | ||
{ | ||
return array.storage(); | ||
} | ||
|
||
template<class ARRAY> | ||
requires(std::is_rvalue_reference_v<ARRAY&&>) | ||
static inline sparrow::arrow_proxy&& extract_arrow_proxy(ARRAY&& array) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same remark here (notice this is just about the return type, the implementation is correct). |
||
{ | ||
return std::move(array).extract_arrow_proxy(); | ||
} | ||
|
||
template<class ARRAY> | ||
requires(std::is_rvalue_reference_v<ARRAY&&>) | ||
static inline auto && extract_array_wrapper(ARRAY&& array) | ||
{ | ||
return std::move(array).extract_array_wrapper(); | ||
} | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,9 @@ | |
|
||
#include <string> // for std::stoull | ||
|
||
|
||
#include "sparrow/arrow_interface/arrow_array.hpp" | ||
#include "sparrow/arrow_interface/arrow_schema.hpp" | ||
#include "sparrow/array_factory.hpp" | ||
#include "sparrow/layout/array_bitmap_base.hpp" | ||
#include "sparrow/layout/array_wrapper.hpp" | ||
|
@@ -26,6 +29,7 @@ | |
#include "sparrow/utils/iterator.hpp" | ||
#include "sparrow/utils/memory.hpp" | ||
#include "sparrow/utils/nullable.hpp" | ||
#include "sparrow/array.hpp" | ||
|
||
namespace sparrow | ||
{ | ||
|
@@ -255,8 +259,15 @@ namespace sparrow | |
fixed_sized_list_array(self_type&&) = default; | ||
fixed_sized_list_array& operator=(self_type&&) = default; | ||
|
||
template<class ...ARGS> | ||
requires(mpl::excludes_copy_and_move_ctor_v<fixed_sized_list_array, ARGS...>) | ||
fixed_sized_list_array(ARGS&& ...args): self_type(create_proxy(std::forward<ARGS>(args)...)) | ||
{} | ||
|
||
private: | ||
|
||
static arrow_proxy create_proxy(std::uint64_t list_size, array && flat_values); | ||
|
||
static uint64_t list_size_from_format(const std::string_view format); | ||
std::pair<offset_type, offset_type> offset_range(size_type i) const; | ||
|
||
|
@@ -491,4 +502,44 @@ namespace sparrow | |
const auto offset = i * m_list_size; | ||
return std::make_pair(offset, offset + m_list_size); | ||
} | ||
|
||
|
||
inline arrow_proxy fixed_sized_list_array::create_proxy(std::uint64_t list_size, array && flat_values) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should allow to pass a range for the null values (similar to what we do here). Same remark for the other layouts where it makes sense. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agree! |
||
{ | ||
const auto size = flat_values.size() / static_cast<std::size_t>(list_size); | ||
|
||
auto wrapper = detail::array_access::extract_array_wrapper(std::move(flat_values)); | ||
auto storage = std::move(*wrapper).extract_arrow_proxy(); | ||
auto flat_schema = storage.extract_schema(); | ||
auto flat_arr = storage.extract_array(); | ||
|
||
const auto bitmap_size = (size + 7 ) / 8; | ||
auto bitmap_ptr = new std::uint8_t[bitmap_size]; | ||
std::fill_n(bitmap_ptr, bitmap_size, static_cast<std::uint8_t>(0xFF) /*all bits 1*/); | ||
|
||
std::string format = "+w:" + std::to_string(list_size); | ||
ArrowSchema schema = make_arrow_schema( | ||
format, | ||
std::nullopt, // name | ||
std::nullopt, // metadata | ||
std::nullopt, // flags, | ||
1, // n_children | ||
new ArrowSchema*[1]{new ArrowSchema(std::move(flat_schema))}, // children | ||
nullptr // dictionary | ||
|
||
); | ||
|
||
ArrowArray arr = make_arrow_array( | ||
static_cast<std::int64_t>(size), // length | ||
0, // null_count | ||
0, // offset | ||
std::vector<buffer<std::uint8_t>>{ | ||
{bitmap_ptr, static_cast<std::size_t>(bitmap_size)}, | ||
}, | ||
1, // n_children | ||
new ArrowArray*[1]{new ArrowArray(std::move(flat_arr))}, // children | ||
nullptr // dictionary | ||
); | ||
return arrow_proxy{std::move(arr), std::move(schema)}; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return type should be a value type instead of an rvalue reference to prevent dangling references.