Skip to content

ARROW-206: Expose a C++ api to compare ranges of slots between two arrays #80

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.

Sign up for GitHub

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

Closed
emkornfield wants to merge 6 commits into from
Closed

ARROW-206: Expose a C++ api to compare ranges of slots between two arrays #80

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dadb244
wip expose range equality to to allow for nested comparisons
emkornfield May 20, 2016
318855d
working primitive tests
emkornfield May 23, 2016
dcbaad4
unittests passing
emkornfield May 23, 2016
f5c6bd5
make format/lint check
emkornfield May 23, 2016
f963639
add in check for null arrays
emkornfield May 23, 2016
d5ae777
remove todo, its handled by type_traits
emkornfield May 23, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions cpp/src/arrow/array-test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,34 @@ TEST_F(TestArray, TestLength) {
ASSERT_EQ(arr->length(), 100);
}

ArrayPtr MakeArrayFromValidBytes(const std::vector<uint8_t>& v, MemoryPool* pool) {
int32_t null_count = v.size() - std::accumulate(v.begin(), v.end(), 0);
std::shared_ptr<Buffer> null_buf = test::bytes_to_null_buffer(v);

BufferBuilder value_builder(pool);
for (size_t i = 0; i < v.size(); ++i) {
value_builder.Append<int32_t>(0);
}

ArrayPtr arr(new Int32Array(v.size(), value_builder.Finish(), null_count, null_buf));
return arr;
}

TEST_F(TestArray, TestEquality) {
auto array = MakeArrayFromValidBytes({1, 0, 1, 1, 0, 1, 0, 0}, pool_);
auto equal_array = MakeArrayFromValidBytes({1, 0, 1, 1, 0, 1, 0, 0}, pool_);
auto unequal_array = MakeArrayFromValidBytes({1, 1, 1, 1, 0, 1, 0, 0}, pool_);

EXPECT_TRUE(array->Equals(array));
EXPECT_TRUE(array->Equals(equal_array));
EXPECT_TRUE(equal_array->Equals(array));
EXPECT_FALSE(equal_array->Equals(unequal_array));
EXPECT_TRUE(array->RangeEquals(4, 8, unequal_array));
EXPECT_FALSE(array->RangeEquals(0, 4, unequal_array));
EXPECT_FALSE(array->RangeEquals(0, 8, unequal_array));
EXPECT_FALSE(array->RangeEquals(1, 2, unequal_array));
}

TEST_F(TestArray, TestIsNull) {
// clang-format off
std::vector<uint8_t> null_bitmap = {1, 0, 1, 1, 0, 1, 0, 0,
Expand Down
14 changes: 14 additions & 0 deletions cpp/src/arrow/array.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,14 @@ bool Array::EqualsExact(const Array& other) const {
return true;
}

bool Array::RangeEqualsExact(int32_t start_idx, int32_t end_idx, const Array& arr) const {
if (this == &arr) { return true; }
for (int i = start_idx; i < end_idx; ++i) {

@emkornfield emkornfield May 23, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you thinking of the case of start_idx == end_idx or something else?

All else being equal it seems that an empty set should equal an empty. Do you disagree?

if (IsNull(i) != arr.IsNull(i)) { return false; }
}
return true;
}

Status Array::Validate() const {
return Status::OK();
}
Expand All @@ -58,4 +66,10 @@ bool NullArray::Equals(const std::shared_ptr<Array>& arr) const {
return arr->length() == length_;
}

bool NullArray::RangeEquals(
int32_t start_idx, int32_t end_idx, const std::shared_ptr<Array>& arr) const {
if (Type::NA != arr->type_enum()) { return false; }
return true;
}

} // namespace arrow
10 changes: 9 additions & 1 deletion cpp/src/arrow/array.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ class Array {

bool EqualsExact(const Array& arr) const;
virtual bool Equals(const std::shared_ptr<Array>& arr) const = 0;

// Compare if the range of slots specified are equal for the given array and
// this array. end_idx exclusive. These methods do not bounds check.
bool RangeEqualsExact(int32_t start_idx, int32_t end_idx, const Array& arr) const;
virtual bool RangeEquals(
int32_t start_idx, int32_t end_idx, const std::shared_ptr<Array>& arr) const = 0;

// Determines if the array is internally consistent. Defaults to always
// returning Status::OK. This can be an expensive check.
virtual Status Validate() const;
Expand All @@ -85,10 +92,11 @@ class NullArray : public Array {
explicit NullArray(int32_t length) : NullArray(std::make_shared<NullType>(), length) {}

bool Equals(const std::shared_ptr<Array>& arr) const override;
bool RangeEquals(int32_t start_idx, int32_t end_idx,
const std::shared_ptr<Array>& arr) const override;
};

typedef std::shared_ptr<Array> ArrayPtr;

} // namespace arrow

#endif
15 changes: 15 additions & 0 deletions cpp/src/arrow/types/list.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,21 @@ bool ListArray::Equals(const std::shared_ptr<Array>& arr) const {
return EqualsExact(*static_cast<const ListArray*>(arr.get()));
}

bool ListArray::RangeEquals(
int32_t start_idx, int32_t end_idx, const std::shared_ptr<Array>& arr) const {
if (this == arr.get()) { return true; }
if (this->type_enum() != arr->type_enum()) { return false; }

@fengguangyuan fengguangyuan May 23, 2016 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do as your've done the check if (!arr) in other place?

auto other = static_cast<ListArray*>(arr.get());

@wesm wesm May 20, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const

for (int i = start_idx; i < end_idx; ++i) {
const bool is_null = IsNull(i);
if ((is_null != arr->IsNull(i)) ||
(!is_null && !values_->RangeEquals(offset(i), offset(i + 1), other->values()))) {
return false;
}
}
return true;
}

Status ListArray::Validate() const {
if (length_ < 0) { return Status::Invalid("Length was negative"); }
if (!offset_buf_) { return Status::Invalid("offset_buf_ was null"); }
Expand Down
3 changes: 3 additions & 0 deletions cpp/src/arrow/types/list.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ class ListArray : public Array {
bool EqualsExact(const ListArray& other) const;
bool Equals(const std::shared_ptr<Array>& arr) const override;

bool RangeEquals(
int32_t start_idx, int32_t end_idx, const ArrayPtr& arr) const override;

protected:
std::shared_ptr<Buffer> offset_buf_;
const int32_t* offsets_;
Expand Down
17 changes: 16 additions & 1 deletion cpp/src/arrow/types/primitive.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,25 @@ bool BooleanArray::EqualsExact(const BooleanArray& other) const {
}
}

bool BooleanArray::Equals(const std::shared_ptr<Array>& arr) const {
bool BooleanArray::Equals(const ArrayPtr& arr) const {
if (this == arr.get()) return true;
if (Type::BOOL != arr->type_enum()) { return false; }
return EqualsExact(*static_cast<const BooleanArray*>(arr.get()));
}

bool BooleanArray::RangeEquals(
int32_t start_idx, int32_t end_idx, const ArrayPtr& arr) const {
if (this == arr.get()) { return true; }
if (!arr) { return false; }
if (this->type_enum() != arr->type_enum()) { return false; }
auto other = static_cast<BooleanArray*>(arr.get());

@wesm wesm May 20, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const

for (int i = start_idx; i < end_idx; ++i) {
bool is_null = IsNull(i);
if (is_null != arr->IsNull(i) || (!is_null && Value(i) != other->Value(i))) {
return false;
}
}
return true;
}

} // namespace arrow
19 changes: 18 additions & 1 deletion cpp/src/arrow/types/primitive.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,21 @@ class PrimitiveArray : public Array {
return PrimitiveArray::EqualsExact(*static_cast<const PrimitiveArray*>(&other)); \
} \
\
bool RangeEquals( \
int32_t start_idx, int32_t end_idx, const ArrayPtr& arr) const override { \
if (this == arr.get()) { return true; } \
if (!arr) { return false; } \
if (this->type_enum() != arr->type_enum()) { return false; } \
auto other = static_cast<NAME*>(arr.get()); \
for (int i = start_idx; i < end_idx; ++i) { \
bool is_null = IsNull(i); \
if (is_null != arr->IsNull(i) || (!is_null && Value(i) != other->Value(i))) { \
return false; \
} \
} \
return true; \
} \
\
const T* raw_data() const { return reinterpret_cast<const T*>(raw_data_); } \
\
T Value(int i) const { return raw_data()[i]; } \
Expand Down Expand Up @@ -248,7 +263,9 @@ class BooleanArray : public PrimitiveArray {
int32_t null_count = 0, const std::shared_ptr<Buffer>& null_bitmap = nullptr);

bool EqualsExact(const BooleanArray& other) const;
bool Equals(const std::shared_ptr<Array>& arr) const override;
bool Equals(const ArrayPtr& arr) const override;
bool RangeEquals(
int32_t start_idx, int32_t end_idx, const ArrayPtr& arr) const override;

const uint8_t* raw_data() const { return reinterpret_cast<const uint8_t*>(raw_data_); }

Expand Down