Skip to content

Commit e519938

Browse files
petrmikheevcopybara-github
authored andcommitted
Verify alloc ids during DataSliceImpl creation.
PiperOrigin-RevId: 715375786 Change-Id: I7b80ab517989c504f9d8b227962ef1e8a02a82ff
1 parent 03fccc4 commit e519938

File tree

3 files changed

+30
-7
lines changed

3 files changed

+30
-7
lines changed

koladata/internal/data_bag_benchmarks.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ void BM_ObjAttributeDataBagManySources(benchmark::State& state) {
209209
for (int j = i; j < batch_size; j += chain_size) {
210210
ds_bldr.Set(j, ds_a.values<ObjectId>()[j]);
211211
}
212-
alloc_ids.Insert(ds.allocation_ids());
212+
alloc_ids.Insert(ds_a.allocation_ids());
213213
}
214214

215215
auto ds =

koladata/internal/data_bag_test.cc

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1307,8 +1307,7 @@ TEST(DataBagTest, InternalSetUnitAttrAndReturnMissingObjectsSparseToDense) {
13071307
for (int64_t sz : {kSize / 100 + 1, kSize}) {
13081308
auto objs_bldr = arolla::DenseArrayBuilder<ObjectId>(sz);
13091309
objs_bldr.Set(0, AllocateSingleObject());
1310-
auto objs = DataSliceImpl::CreateWithAllocIds(
1311-
ds.allocation_ids(), std::move(objs_bldr).Build());
1310+
auto objs = DataSliceImpl::Create(std::move(objs_bldr).Build());
13121311
ASSERT_OK_AND_ASSIGN(
13131312
auto _, db->InternalSetUnitAttrAndReturnMissingObjects(objs, "a"));
13141313
}
@@ -1438,8 +1437,8 @@ TEST(DataBagTest, SetGetDataSliceUUid) {
14381437
auto object_array = arolla::CreateFullDenseArray<ObjectId>(
14391438
{alloc1.ObjectByOffset(5), alloc3.ObjectByOffset(0),
14401439
alloc2.ObjectByOffset(9)});
1441-
ds_b = DataSliceImpl::CreateWithAllocIds(AllocationIdSet({alloc1, alloc2}),
1442-
object_array);
1440+
ds_b = DataSliceImpl::CreateWithAllocIds(
1441+
AllocationIdSet({alloc1, alloc2, alloc3}), object_array);
14431442
ASSERT_OK(db->SetAttr(ds, "b", ds_b));
14441443
ASSERT_OK_AND_ASSIGN(ds_b_get, db->GetAttr(ds, "b"));
14451444
ASSERT_EQ(ds_b_get.dtype(), arolla::GetQType<ObjectId>());

koladata/internal/data_slice.h

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,27 @@ bool VerifyNonIntersectingIds(const arolla::DenseArray<T>& main_values,
325325
return !(process_ids(values) || ...);
326326
}
327327

328+
template <class T>
329+
bool VerifyAllocIds(const AllocationIdSet& alloc_ids,
330+
const arolla::DenseArray<T>& values) {
331+
if constexpr (std::is_same_v<T, ObjectId>) {
332+
bool res = true;
333+
values.ForEachPresent([&](int64_t /*id*/, ObjectId obj_id) {
334+
if (!obj_id.IsSmallAlloc()) {
335+
for (AllocationId alloc_id : alloc_ids.ids()) {
336+
if (alloc_id.Contains(obj_id)) return;
337+
}
338+
res = false;
339+
} else if (!alloc_ids.contains_small_allocation_id()) {
340+
res = false;
341+
}
342+
});
343+
return res;
344+
} else {
345+
return true;
346+
}
347+
}
348+
328349
template <class T>
329350
constexpr bool AreAllTypesDistinct(std::type_identity<T>) {
330351
return true;
@@ -396,27 +417,30 @@ DataSliceImpl DataSliceImpl::CreateWithAllocIds(
396417
AllocationIdSet allocation_ids, arolla::DenseArray<T> main_values,
397418
arolla::DenseArray<Ts>... values) {
398419
DataSliceImpl res;
399-
CreateImpl(res, std::move(main_values), std::move(values)...);
400420
if constexpr ((std::is_same_v<T, ObjectId> || ... ||
401421
std::is_same_v<Ts, ObjectId>)) {
422+
DCHECK(data_slice_impl::VerifyAllocIds(allocation_ids, main_values));
423+
DCHECK((data_slice_impl::VerifyAllocIds(allocation_ids, values) && ...));
402424
res.internal_->allocation_ids = std::move(allocation_ids);
403425
} else {
404426
(void)allocation_ids;
405427
}
428+
CreateImpl(res, std::move(main_values), std::move(values)...);
406429
return res;
407430
}
408431

409432
template <class T>
410433
DataSliceImpl DataSliceImpl::CreateWithTypesBuffer(TypesBuffer types_buffer,
411434
AllocationIdSet allocation_ids, arolla::DenseArray<T> values) {
412435
DataSliceImpl res;
413-
CreateImpl(res, std::move(values));
414436
res.internal_->types_buffer = std::move(types_buffer);
415437
if constexpr (std::is_same_v<T, ObjectId>) {
438+
DCHECK(data_slice_impl::VerifyAllocIds(allocation_ids, values));
416439
res.internal_->allocation_ids = std::move(allocation_ids);
417440
} else {
418441
(void)allocation_ids;
419442
}
443+
CreateImpl(res, std::move(values));
420444
return res;
421445
}
422446

0 commit comments

Comments
 (0)