paleolimbot commented on code in PR #702:
URL: https://github.com/apache/arrow-nanoarrow/pull/702#discussion_r1905851129
##########
src/nanoarrow/hpp/view.hpp:
##########
@@ -79,7 +80,7 @@ struct RandomAccessRange {
};
const_iterator begin() const { return {0, this}; }
- const_iterator end() const { return {size, this}; }
+ const_iterator end() const { return {size - offset, this}; }
Review Comment:
I would have expected this to be:
```c
const_iterator begin() const { return {offset, this}; }
const_iterator end() const { return {offset + size, this}; }
```
...or to have it be `[0, size]` and to have the offset handled by
`range->get()`.
##########
src/nanoarrow/hpp/view.hpp:
##########
@@ -157,6 +158,7 @@ class ViewArrayAs {
array_view->buffer_views[1].data.data,
array_view->offset,
},
+ array_view->offset,
Review Comment:
This seems like the right thing to put here (at least one of Range or Get
need it?)
##########
src/nanoarrow/hpp/view_test.cc:
##########
@@ -135,3 +135,109 @@ TEST(NanoarrowHppTest, NanoarrowHppViewArrayStreamTest) {
EXPECT_EQ(stream_view.code(), ENOMEM);
EXPECT_STREQ(stream_view.error()->message, "foo bar");
}
+
+TEST(NanoarrowHppTest, NanoarrowHppViewArrayOffsetTest) {
+ nanoarrow::UniqueSchema schema{};
+ ArrowSchemaInit(schema.get());
+ ASSERT_EQ(ArrowSchemaSetType(schema.get(), NANOARROW_TYPE_INT32),
NANOARROW_OK);
+
+ nanoarrow::UniqueArray array{};
+ ASSERT_EQ(ArrowArrayInitFromSchema(array.get(), schema.get(), nullptr),
NANOARROW_OK);
+ ASSERT_EQ(ArrowArrayStartAppending(array.get()), NANOARROW_OK);
+ ASSERT_EQ(ArrowArrayAppendInt(array.get(), 0), NANOARROW_OK);
+ ASSERT_EQ(ArrowArrayAppendInt(array.get(), 1), NANOARROW_OK);
+ ASSERT_EQ(ArrowArrayAppendInt(array.get(), 2), NANOARROW_OK);
+ ASSERT_EQ(ArrowArrayAppendInt(array.get(), 3), NANOARROW_OK);
+ ASSERT_EQ(ArrowArrayFinishBuildingDefault(array.get(), nullptr),
NANOARROW_OK);
+ array->offset = 2;
+
+ int32_t expected[] = {2, 3};
+ int i = 0;
+ for (auto slot : nanoarrow::ViewArrayAs<int32_t>(array.get())) {
+ EXPECT_EQ(slot, expected[i]);
+ i++;
+ }
+
+ nanoarrow::UniqueArrayView array_view{};
+ ASSERT_EQ(ArrowArrayViewInitFromSchema(array_view.get(), schema.get(),
nullptr),
+ NANOARROW_OK);
+ ASSERT_EQ(ArrowArrayViewSetArray(array_view.get(), array.get(), nullptr),
NANOARROW_OK);
+ i = 0;
+ for (auto slot : nanoarrow::ViewArrayAs<int32_t>(array_view.get())) {
+ EXPECT_EQ(slot, expected[i]);
+ i++;
+ }
+}
+
+TEST(NanoarrowHppTest, NanoarrowHppViewArrayAsBytesOffsetTest) {
+ using namespace nanoarrow::literals;
+
+ nanoarrow::UniqueSchema schema{};
+ ArrowSchemaInit(schema.get());
+ ASSERT_EQ(ArrowSchemaSetType(schema.get(), NANOARROW_TYPE_STRING),
NANOARROW_OK);
+
+ nanoarrow::UniqueArray array{};
+ ASSERT_EQ(ArrowArrayInitFromSchema(array.get(), schema.get(), nullptr),
NANOARROW_OK);
+ ASSERT_EQ(ArrowArrayStartAppending(array.get()), NANOARROW_OK);
+ ASSERT_EQ(ArrowArrayAppendString(array.get(), "foo"_asv), NANOARROW_OK);
+ ASSERT_EQ(ArrowArrayAppendString(array.get(), "bar"_asv), NANOARROW_OK);
+ ASSERT_EQ(ArrowArrayAppendString(array.get(), "baz"_asv), NANOARROW_OK);
+ ASSERT_EQ(ArrowArrayAppendString(array.get(), "qux"_asv), NANOARROW_OK);
+ ASSERT_EQ(ArrowArrayFinishBuildingDefault(array.get(), nullptr),
NANOARROW_OK);
+ array->offset = 2;
+
+ ArrowStringView expected[] = {"baz"_asv, "qux"_asv};
+ int i = 0;
+ for (auto slot : nanoarrow::ViewArrayAsBytes<32>(array.get())) {
+ EXPECT_EQ(slot, expected[i]);
+ i++;
+ }
+
+ nanoarrow::UniqueArrayView array_view{};
+ ASSERT_EQ(ArrowArrayViewInitFromSchema(array_view.get(), schema.get(),
nullptr),
+ NANOARROW_OK);
+ ASSERT_EQ(ArrowArrayViewSetArray(array_view.get(), array.get(), nullptr),
NANOARROW_OK);
+ i = 0;
+ for (auto slot : nanoarrow::ViewArrayAsBytes<32>(array_view.get())) {
+ EXPECT_EQ(slot, expected[i]);
+ i++;
+ }
+}
+
+TEST(NanoarrowHppTest, NanoarrowHppViewArrayAsFixedSizeBytesOffsetTest) {
+ using namespace nanoarrow::literals;
+
+ constexpr int32_t FixedSize = 3;
+ nanoarrow::UniqueSchema schema{};
+ ArrowSchemaInit(schema.get());
+ ASSERT_EQ(ArrowSchemaSetTypeFixedSize(schema.get(),
NANOARROW_TYPE_FIXED_SIZE_BINARY,
+ FixedSize),
+ NANOARROW_OK);
+
+ nanoarrow::UniqueArray array{};
+ ASSERT_EQ(ArrowArrayInitFromSchema(array.get(), schema.get(), nullptr),
NANOARROW_OK);
+ ASSERT_EQ(ArrowArrayStartAppending(array.get()), NANOARROW_OK);
+ ASSERT_EQ(ArrowArrayAppendBytes(array.get(), {{"foo"}, FixedSize}),
NANOARROW_OK);
+ ASSERT_EQ(ArrowArrayAppendBytes(array.get(), {{"bar"}, FixedSize}),
NANOARROW_OK);
+ ASSERT_EQ(ArrowArrayAppendBytes(array.get(), {{"baz"}, FixedSize}),
NANOARROW_OK);
+ ASSERT_EQ(ArrowArrayAppendBytes(array.get(), {{"qux"}, FixedSize}),
NANOARROW_OK);
+ ASSERT_EQ(ArrowArrayFinishBuildingDefault(array.get(), nullptr),
NANOARROW_OK);
+ array->offset = 2;
+
+ ArrowStringView expected[] = {"baz"_asv, "qux"_asv};
+ int i = 0;
+ for (auto slot : nanoarrow::ViewArrayAsFixedSizeBytes(array.get(),
FixedSize)) {
+ EXPECT_EQ(slot, expected[i]);
+ i++;
+ }
Review Comment:
Can you use `::testing::ElementsAre("baz"_asv, "qux"_asv)`? (Not sure if the
templates work for string views but they might)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
