diff --git a/internal/core/src/mmap/ChunkedColumn.h b/internal/core/src/mmap/ChunkedColumn.h index d4e38d2f90..8368877217 100644 --- a/internal/core/src/mmap/ChunkedColumn.h +++ b/internal/core/src/mmap/ChunkedColumn.h @@ -175,6 +175,10 @@ class ChunkedColumnBase : public ChunkedColumnInterface { fn(true, i); } } + // Non-nullable is fully handled above; without this return the + // control falls through into the nullable block and invokes fn a + // second time per row. + return; } // nullable: if (offsets == nullptr) { diff --git a/internal/core/src/mmap/ChunkedColumnGroup.h b/internal/core/src/mmap/ChunkedColumnGroup.h index a5b3d1b2bf..f235c7f7e5 100644 --- a/internal/core/src/mmap/ChunkedColumnGroup.h +++ b/internal/core/src/mmap/ChunkedColumnGroup.h @@ -269,6 +269,10 @@ class ProxyChunkColumn : public ChunkedColumnInterface { fn(true, i); } } + // Non-nullable is fully handled above; without this return the + // control falls through into the nullable block and invokes fn a + // second time per row. + return; } // nullable: if (count == 0) { diff --git a/internal/core/src/mmap/ChunkedColumnGroupTest.cpp b/internal/core/src/mmap/ChunkedColumnGroupTest.cpp index d653eb9ef3..4c0143c38b 100644 --- a/internal/core/src/mmap/ChunkedColumnGroupTest.cpp +++ b/internal/core/src/mmap/ChunkedColumnGroupTest.cpp @@ -274,6 +274,29 @@ TEST_F(ChunkedColumnGroupTest, ProxyChunkColumn) { &offset, 1); + // Regression: a non-nullable column must invoke the callback EXACTLY once + // per row. Before the missing-return fix, the !nullable_ branch fell + // through into the nullable branch and invoked the callback a second time + // per row. + { + std::vector all_offsets = {0, 1, 2, 3, 4}; + std::unordered_map call_count; + proxy_int64->BulkIsValid( + nullptr, + [&](bool is_valid, size_t i) { + EXPECT_TRUE(is_valid); + call_count[i]++; + }, + all_offsets.data(), + static_cast(all_offsets.size())); + ASSERT_EQ(call_count.size(), all_offsets.size()); + for (const auto& kv : call_count) { + EXPECT_EQ(kv.second, 1) + << "row " << kv.first << " callback invoked " << kv.second + << " times"; + } + } + // Test string proxy auto proxy_string = std::make_shared( column_group, FieldId(2), string_field_meta);