Skip to content

Commit 7a80870

Browse files
WillAydWillAyd
authored
fix(cpp): Fix offset handling in ViewArrayAs Range Helpers (#702)
1 parent 7844799 commit 7a80870

File tree

3 files changed

+98
-15
lines changed

3 files changed

+98
-15
lines changed

‎src/nanoarrow/common/utils_test.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -739,7 +739,7 @@ TEST(RandomAccessRangeTest, ConstructionAndPrinting) {
739739
auto square = [](int64_t i) { return i * i; };
740740

741741
// the range is usable as a constant
742-
const nanoarrow::internal::RandomAccessRange<decltype(square)> squares{square, 4};
742+
const nanoarrow::internal::RandomAccessRange<decltype(square)> squares{square, 0, 4};
743743

744744
// gtest recognizes the range as a container and can print it
745745
EXPECT_THAT(squares, testing::ElementsAre(0, 1, 4, 9));

‎src/nanoarrow/hpp/view.hpp

+9-14
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ class Maybe {
6565
template <typename Get>
6666
struct RandomAccessRange {
6767
Get get;
68+
int64_t offset;
6869
int64_t size;
6970

7071
using value_type = decltype(std::declval<Get>()(0));
@@ -78,8 +79,8 @@ struct RandomAccessRange {
7879
value_type operator*() const { return range->get(i); }
7980
};
8081

81-
const_iterator begin() const { return {0, this}; }
82-
const_iterator end() const { return {size, this}; }
82+
const_iterator begin() const { return {offset, this}; }
83+
const_iterator end() const { return {offset + size, this}; }
8384
};
8485

8586
template <typename Next>
@@ -132,10 +133,8 @@ class ViewArrayAs {
132133
struct Get {
133134
const uint8_t* validity;
134135
const void* values;
135-
int64_t offset;
136136

137137
internal::Maybe<T> operator()(int64_t i) const {
138-
i += offset;
139138
if (validity == nullptr || ArrowBitGet(validity, i)) {
140139
if (std::is_same<T, bool>::value) {
141140
return ArrowBitGet(static_cast<const uint8_t*>(values), i);
@@ -155,8 +154,8 @@ class ViewArrayAs {
155154
Get{
156155
array_view->buffer_views[0].data.as_uint8,
157156
array_view->buffer_views[1].data.data,
158-
array_view->offset,
159157
},
158+
array_view->offset,
160159
array_view->length,
161160
} {}
162161

@@ -165,8 +164,8 @@ class ViewArrayAs {
165164
Get{
166165
static_cast<const uint8_t*>(array->buffers[0]),
167166
array->buffers[1],
168-
/*offset=*/0,
169167
},
168+
array->offset,
170169
array->length,
171170
} {}
172171

@@ -193,10 +192,8 @@ class ViewArrayAsBytes {
193192
const uint8_t* validity;
194193
const void* offsets;
195194
const char* data;
196-
int64_t offset;
197195

198196
internal::Maybe<ArrowStringView> operator()(int64_t i) const {
199-
i += offset;
200197
auto* offsets = static_cast<const OffsetType*>(this->offsets);
201198
if (validity == nullptr || ArrowBitGet(validity, i)) {
202199
return ArrowStringView{data + offsets[i], offsets[i + 1] - offsets[i]};
@@ -214,8 +211,8 @@ class ViewArrayAsBytes {
214211
array_view->buffer_views[0].data.as_uint8,
215212
array_view->buffer_views[1].data.data,
216213
array_view->buffer_views[2].data.as_char,
217-
array_view->offset,
218214
},
215+
array_view->offset,
219216
array_view->length,
220217
} {}
221218

@@ -225,8 +222,8 @@ class ViewArrayAsBytes {
225222
static_cast<const uint8_t*>(array->buffers[0]),
226223
array->buffers[1],
227224
static_cast<const char*>(array->buffers[2]),
228-
/*offset=*/0,
229225
},
226+
array->offset,
230227
array->length,
231228
} {}
232229

@@ -246,11 +243,9 @@ class ViewArrayAsFixedSizeBytes {
246243
struct Get {
247244
const uint8_t* validity;
248245
const char* data;
249-
int64_t offset;
250246
int fixed_size;
251247

252248
internal::Maybe<ArrowStringView> operator()(int64_t i) const {
253-
i += offset;
254249
if (validity == nullptr || ArrowBitGet(validity, i)) {
255250
return ArrowStringView{data + i * fixed_size, fixed_size};
256251
}
@@ -266,9 +261,9 @@ class ViewArrayAsFixedSizeBytes {
266261
Get{
267262
array_view->buffer_views[0].data.as_uint8,
268263
array_view->buffer_views[1].data.as_char,
269-
array_view->offset,
270264
fixed_size,
271265
},
266+
array_view->offset,
272267
array_view->length,
273268
} {}
274269

@@ -277,9 +272,9 @@ class ViewArrayAsFixedSizeBytes {
277272
Get{
278273
static_cast<const uint8_t*>(array->buffers[0]),
279274
static_cast<const char*>(array->buffers[1]),
280-
/*offset=*/0,
281275
fixed_size,
282276
},
277+
array->offset,
283278
array->length,
284279
} {}
285280

‎src/nanoarrow/hpp/view_test.cc

+88
Original file line numberDiff line numberDiff line change
@@ -135,3 +135,91 @@ TEST(NanoarrowHppTest, NanoarrowHppViewArrayStreamTest) {
135135
EXPECT_EQ(stream_view.code(), ENOMEM);
136136
EXPECT_STREQ(stream_view.error()->message, "foo bar");
137137
}
138+
139+
TEST(NanoarrowHppTest, NanoarrowHppViewArrayOffsetTest) {
140+
nanoarrow::UniqueSchema schema{};
141+
ArrowSchemaInit(schema.get());
142+
ASSERT_EQ(ArrowSchemaSetType(schema.get(), NANOARROW_TYPE_INT32), NANOARROW_OK);
143+
144+
nanoarrow::UniqueArray array{};
145+
ASSERT_EQ(ArrowArrayInitFromSchema(array.get(), schema.get(), nullptr), NANOARROW_OK);
146+
ASSERT_EQ(ArrowArrayStartAppending(array.get()), NANOARROW_OK);
147+
ASSERT_EQ(ArrowArrayAppendInt(array.get(), 0), NANOARROW_OK);
148+
ASSERT_EQ(ArrowArrayAppendInt(array.get(), 1), NANOARROW_OK);
149+
ASSERT_EQ(ArrowArrayAppendInt(array.get(), 2), NANOARROW_OK);
150+
ASSERT_EQ(ArrowArrayAppendInt(array.get(), 3), NANOARROW_OK);
151+
ASSERT_EQ(ArrowArrayFinishBuildingDefault(array.get(), nullptr), NANOARROW_OK);
152+
array->offset = 2;
153+
array->length = 2;
154+
155+
EXPECT_THAT(nanoarrow::ViewArrayAs<int32_t>(array.get()), testing::ElementsAre(2, 3));
156+
157+
nanoarrow::UniqueArrayView array_view{};
158+
ASSERT_EQ(ArrowArrayViewInitFromSchema(array_view.get(), schema.get(), nullptr),
159+
NANOARROW_OK);
160+
ASSERT_EQ(ArrowArrayViewSetArray(array_view.get(), array.get(), nullptr), NANOARROW_OK);
161+
162+
EXPECT_THAT(nanoarrow::ViewArrayAs<int32_t>(array_view.get()),
163+
testing::ElementsAre(2, 3));
164+
}
165+
166+
TEST(NanoarrowHppTest, NanoarrowHppViewArrayAsBytesOffsetTest) {
167+
using namespace nanoarrow::literals;
168+
169+
nanoarrow::UniqueSchema schema{};
170+
ArrowSchemaInit(schema.get());
171+
ASSERT_EQ(ArrowSchemaSetType(schema.get(), NANOARROW_TYPE_STRING), NANOARROW_OK);
172+
173+
nanoarrow::UniqueArray array{};
174+
ASSERT_EQ(ArrowArrayInitFromSchema(array.get(), schema.get(), nullptr), NANOARROW_OK);
175+
ASSERT_EQ(ArrowArrayStartAppending(array.get()), NANOARROW_OK);
176+
ASSERT_EQ(ArrowArrayAppendString(array.get(), "foo"_asv), NANOARROW_OK);
177+
ASSERT_EQ(ArrowArrayAppendString(array.get(), "bar"_asv), NANOARROW_OK);
178+
ASSERT_EQ(ArrowArrayAppendString(array.get(), "baz"_asv), NANOARROW_OK);
179+
ASSERT_EQ(ArrowArrayAppendString(array.get(), "qux"_asv), NANOARROW_OK);
180+
ASSERT_EQ(ArrowArrayFinishBuildingDefault(array.get(), nullptr), NANOARROW_OK);
181+
array->offset = 2;
182+
array->length = 2;
183+
184+
EXPECT_THAT(nanoarrow::ViewArrayAsBytes<32>(array.get()),
185+
testing::ElementsAre("baz"_asv, "qux"_asv));
186+
187+
nanoarrow::UniqueArrayView array_view{};
188+
ASSERT_EQ(ArrowArrayViewInitFromSchema(array_view.get(), schema.get(), nullptr),
189+
NANOARROW_OK);
190+
ASSERT_EQ(ArrowArrayViewSetArray(array_view.get(), array.get(), nullptr), NANOARROW_OK);
191+
EXPECT_THAT(nanoarrow::ViewArrayAsBytes<32>(array_view.get()),
192+
testing::ElementsAre("baz"_asv, "qux"_asv));
193+
}
194+
195+
TEST(NanoarrowHppTest, NanoarrowHppViewArrayAsFixedSizeBytesOffsetTest) {
196+
using namespace nanoarrow::literals;
197+
198+
constexpr int32_t FixedSize = 3;
199+
nanoarrow::UniqueSchema schema{};
200+
ArrowSchemaInit(schema.get());
201+
ASSERT_EQ(ArrowSchemaSetTypeFixedSize(schema.get(), NANOARROW_TYPE_FIXED_SIZE_BINARY,
202+
FixedSize),
203+
NANOARROW_OK);
204+
205+
nanoarrow::UniqueArray array{};
206+
ASSERT_EQ(ArrowArrayInitFromSchema(array.get(), schema.get(), nullptr), NANOARROW_OK);
207+
ASSERT_EQ(ArrowArrayStartAppending(array.get()), NANOARROW_OK);
208+
ASSERT_EQ(ArrowArrayAppendBytes(array.get(), {{"foo"}, FixedSize}), NANOARROW_OK);
209+
ASSERT_EQ(ArrowArrayAppendBytes(array.get(), {{"bar"}, FixedSize}), NANOARROW_OK);
210+
ASSERT_EQ(ArrowArrayAppendBytes(array.get(), {{"baz"}, FixedSize}), NANOARROW_OK);
211+
ASSERT_EQ(ArrowArrayAppendBytes(array.get(), {{"qux"}, FixedSize}), NANOARROW_OK);
212+
ASSERT_EQ(ArrowArrayFinishBuildingDefault(array.get(), nullptr), NANOARROW_OK);
213+
array->offset = 2;
214+
array->length = 2;
215+
216+
EXPECT_THAT(nanoarrow::ViewArrayAsFixedSizeBytes(array.get(), FixedSize),
217+
testing::ElementsAre("baz"_asv, "qux"_asv));
218+
219+
nanoarrow::UniqueArrayView array_view{};
220+
ASSERT_EQ(ArrowArrayViewInitFromSchema(array_view.get(), schema.get(), nullptr),
221+
NANOARROW_OK);
222+
ASSERT_EQ(ArrowArrayViewSetArray(array_view.get(), array.get(), nullptr), NANOARROW_OK);
223+
EXPECT_THAT(nanoarrow::ViewArrayAsFixedSizeBytes(array_view.get(), FixedSize),
224+
testing::ElementsAre("baz"_asv, "qux"_asv));
225+
}

0 commit comments

Comments
 (0)