Skip to content

Commit

Permalink
Do not instantiate children pointer if there is no children
Browse files Browse the repository at this point in the history
  • Loading branch information
@Alex-PLACET
Alex-PLACET committed Apr 30, 2024
1 parent 3182775 commit 1a4b420
Show file tree
Hide file tree
Showing 2 changed files with 147 additions and 13 deletions.
34 changes: 26 additions & 8 deletions include/sparrow/c_interface.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,13 @@ namespace sparrow
)
{
SPARROW_ASSERT_FALSE(format.empty())
SPARROW_ASSERT_TRUE(std::ranges::none_of(children, [](const auto& child) { return child == nullptr; }))
SPARROW_ASSERT_TRUE(std::ranges::none_of(
children,
[](const auto& child)
{
return child == nullptr;
}
))

auto schema = std::make_unique<ArrowSchema>();
schema->private_data = new ArrowSchemaPrivateData<Allocator>;
Expand Down Expand Up @@ -310,10 +316,13 @@ namespace sparrow

schema->flags = flags.has_value() ? static_cast<int64_t>(flags.value()) : 0;
schema->n_children = children.size();
schema->children = new ArrowSchema*[schema->n_children];
for (int64_t i = 0; i < schema->n_children; ++i)
if (schema->n_children > 0)
{
schema->children[i] = children[i].release();
schema->children = new ArrowSchema*[schema->n_children];
for (int64_t i = 0; i < schema->n_children; ++i)
{
schema->children[i] = children[i].release();
}
}
schema->dictionary = dictionary.release();
return schema;
Expand Down Expand Up @@ -356,7 +365,13 @@ namespace sparrow
SPARROW_ASSERT_TRUE(null_count >= -1)
SPARROW_ASSERT_TRUE(offset >= 0)
SPARROW_ASSERT_TRUE(n_buffers >= 0)
SPARROW_ASSERT_TRUE(std::ranges::none_of(children, [](const auto& child) { return child == nullptr; }))
SPARROW_ASSERT_TRUE(std::ranges::none_of(
children,
[](const auto& child)
{
return child == nullptr;
}
))

auto array = std::make_unique<ArrowArray>();
array->private_data = new ArrowArrayPrivateData<T, Allocator>;
Expand All @@ -373,10 +388,13 @@ namespace sparrow
->buffer_allocator.allocate(buffers_size);
}
array->n_children = children.size();
array->children = new ArrowArray*[array->n_children];
for (int64_t i = 0; i < array->n_children; ++i)
if (array->n_children > 0)
{
array->children[i] = children[i].release();
array->children = new ArrowArray*[array->n_children];
for (int64_t i = 0; i < array->n_children; ++i)
{
array->children[i] = children[i].release();
}
}
array->dictionary = dictionary.release();
return array;
Expand Down
126 changes: 121 additions & 5 deletions test/test_c_data_interface.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,14 @@ TEST_SUITE("C Data Interface")
auto dictionary = std::make_unique<ArrowArray>();
const auto dictionary_ptr = dictionary.get();

const auto array = sparrow::make_array_constructor<int, std::allocator>(1, 0, 0, 1, children, std::move(dictionary));
const auto array = sparrow::make_array_constructor<int, std::allocator>(
1,
0,
0,
1,
children,
std::move(dictionary)
);
CHECK_EQ(array->length, 1);
CHECK_EQ(array->null_count, 0);
CHECK_EQ(array->offset, 0);
Expand All @@ -51,13 +58,68 @@ TEST_SUITE("C Data Interface")
CHECK_NE(array->private_data, nullptr);
}

SUBCASE("make_array_constructor no children and dictionary")
{
std::vector<std::unique_ptr<ArrowArray>> children;
const auto array = sparrow::make_array_constructor<int, std::allocator>(
1,
0,
0,
1,
children,
std::move(std::unique_ptr<ArrowArray>())
);
CHECK_EQ(array->length, 1);
CHECK_EQ(array->null_count, 0);
CHECK_EQ(array->offset, 0);
CHECK_EQ(array->n_buffers, 1);
CHECK_EQ(array->n_children, 0);
CHECK_NE(array->buffers, nullptr);
CHECK_EQ(array->children, nullptr);
CHECK_EQ(array->dictionary, nullptr);
CHECK_EQ(array->release, sparrow::delete_array<int, std::allocator>);
CHECK_NE(array->private_data, nullptr);
}

SUBCASE("ArrowArray release")
{
std::vector<std::unique_ptr<ArrowArray>> children;
children.emplace_back(new ArrowArray);
children.emplace_back(new ArrowArray);
auto dictionary = std::make_unique<ArrowArray>();
auto array = sparrow::make_array_constructor<int, std::allocator>(1, 0, 0, 1, children, std::move(dictionary));
auto array = sparrow::make_array_constructor<int, std::allocator>(
1,
0,
0,
1,
children,
std::move(dictionary)
);

array->release(array.get());

CHECK_EQ(array->length, 0);
CHECK_EQ(array->null_count, 0);
CHECK_EQ(array->offset, 0);
CHECK_EQ(array->n_buffers, 0);
CHECK_EQ(array->n_children, 0);
CHECK_EQ(array->buffers, nullptr);
CHECK_EQ(array->children, nullptr);
CHECK_EQ(array->release, nullptr);
CHECK_EQ(array->private_data, nullptr);
}

SUBCASE("ArrowArray release no children and dictionary")
{
std::vector<std::unique_ptr<ArrowArray>> children;
auto array = sparrow::make_array_constructor<int, std::allocator>(
1,
0,
0,
1,
children,
std::move(std::unique_ptr<ArrowArray>())
);

array->release(array.get());

Expand All @@ -73,6 +135,7 @@ TEST_SUITE("C Data Interface")
}
}


TEST_CASE("ArrowSchema")
{
SUBCASE("make_schema_constructor")
Expand Down Expand Up @@ -101,9 +164,14 @@ TEST_SUITE("C Data Interface")
);

const auto schema_format = std::string_view(schema->format);
bool lol = (schema_format == format);
// CHECK_EQ(std::string_view(schema->name), name);
// CHECK_EQ(std::string_view(schema->metadata), metadata);
const bool format_eq = schema_format == format;
CHECK(format_eq);
const auto schema_name = std::string_view(schema->name);
const bool name_eq = schema_name == name;
CHECK(name_eq);
const auto schema_metadata = std::string_view(schema->metadata);
const bool metadata_eq = schema_metadata == metadata;
CHECK(metadata_eq);
CHECK_EQ(schema->flags, 1);
CHECK_EQ(schema->n_children, 2);
REQUIRE_NE(schema->children, nullptr);
Expand All @@ -114,6 +182,31 @@ TEST_SUITE("C Data Interface")
CHECK_NE(schema->private_data, nullptr);
}

SUBCASE("make_schema_constructor no children, no dictionary, no name and metadata")
{
std::vector<std::unique_ptr<ArrowSchema>> children;
const auto schema = sparrow::make_arrow_schema<std::allocator>(
"format",
std::string_view(),
std::string_view(),
sparrow::ArrowFlag::DICTIONARY_ORDERED,
children,
std::move(std::unique_ptr<ArrowSchema>())
);

const auto schema_format = std::string_view(schema->format);
const bool format_eq = schema_format == "format";
CHECK(format_eq);
CHECK_EQ(schema->name, nullptr);
CHECK_EQ(schema->metadata, nullptr);
CHECK_EQ(schema->flags, 1);
CHECK_EQ(schema->n_children, 0);
CHECK_EQ(schema->children, nullptr);
CHECK_EQ(schema->dictionary, nullptr);
CHECK_EQ(schema->release, sparrow::delete_schema<std::allocator>);
CHECK_NE(schema->private_data, nullptr);
}

SUBCASE("ArrowSchema release")
{
std::vector<std::unique_ptr<ArrowSchema>> children;
Expand All @@ -140,5 +233,28 @@ TEST_SUITE("C Data Interface")
CHECK_EQ(schema->release, nullptr);
CHECK_EQ(schema->private_data, nullptr);
}

SUBCASE("ArrowSchema release no children, no dictionary, no name and metadata")
{
std::vector<std::unique_ptr<ArrowSchema>> children;
auto schema = sparrow::make_arrow_schema<std::allocator>(
"format",
std::string_view(),
std::string_view(),
sparrow::ArrowFlag::DICTIONARY_ORDERED,
children,
std::move(std::unique_ptr<ArrowSchema>())
);

schema->release(schema.get());

CHECK_EQ(schema->format, nullptr);
CHECK_EQ(schema->name, nullptr);
CHECK_EQ(schema->metadata, nullptr);
CHECK_EQ(schema->children, nullptr);
CHECK_EQ(schema->dictionary, nullptr);
CHECK_EQ(schema->release, nullptr);
CHECK_EQ(schema->private_data, nullptr);
}
}
}

0 comments on commit 1a4b420

Please sign in to comment.