fix: BulkIsValid non-nullable fall-through in ChunkedColumn/ProxyChunkColumn (#51598)

The `if (!nullable_)` branch had no `return`, so a non-nullable column
fell through into the nullable block and invoked the callback a second
time per row (latent — currently masked by callers that guard with `if
(!IsNullable()) return`). Added the missing `return`.

issue: #51385

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: xiaofanluan <xf@hjjaq.com>
Co-authored-by: xiaofanluan <xf@hjjaq.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
James
2026-07-20 23:40:43 +08:00
committed by GitHub
co-authored by xiaofanluan Claude Opus 4.8
parent fd46aa32d1
commit bf674af67b
3 changed files with 31 additions and 0 deletions
+4
View File
@@ -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) {
@@ -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) {
@@ -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<int64_t> all_offsets = {0, 1, 2, 3, 4};
std::unordered_map<size_t, int> call_count;
proxy_int64->BulkIsValid(
nullptr,
[&](bool is_valid, size_t i) {
EXPECT_TRUE(is_valid);
call_count[i]++;
},
all_offsets.data(),
static_cast<int64_t>(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<ProxyChunkColumn>(
column_group, FieldId(2), string_field_meta);