Skip to content

Commit

Permalink
Updated ColumnString::LoadBody
Browse files Browse the repository at this point in the history
made it more thread-safe,
removed piece of code that confused lots of static analyzers (false positive on dereferencing nullptr-`block` variable)
  • Loading branch information
Enmk committed May 24, 2023
1 parent 1350915 commit 1f745e6
Showing 1 changed file with 19 additions and 8 deletions.
27 changes: 19 additions & 8 deletions clickhouse/columns/string.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -252,27 +252,38 @@ void ColumnString::Append(ColumnRef column) {
}

bool ColumnString::LoadBody(InputStream* input, size_t rows) {
items_.clear();
blocks_.clear();
if (rows == 0) {
items_.clear();
blocks_.clear();

return true;
}

items_.reserve(rows);
Block * block = nullptr;
decltype(items_) new_items;
decltype(blocks_) new_blocks;

new_items.reserve(rows);

// Suboptimzal if the first row string is >DEFAULT_BLOCK_SIZE, but that must be a very rare case.
Block * block = &new_blocks.emplace_back(DEFAULT_BLOCK_SIZE);

// TODO(performance): unroll a loop to a first row (to get rid of `blocks_.size() == 0` check) and the rest.
for (size_t i = 0; i < rows; ++i) {
uint64_t len;
if (!WireFormat::ReadUInt64(*input, &len))
return false;

if (blocks_.size() == 0 || len > block->GetAvailable())
block = &blocks_.emplace_back(std::max<size_t>(DEFAULT_BLOCK_SIZE, len));
if (len > block->GetAvailable())
block = &new_blocks.emplace_back(std::max<size_t>(DEFAULT_BLOCK_SIZE, len));

if (!WireFormat::ReadBytes(*input, block->GetCurrentWritePos(), len))
return false;

items_.emplace_back(block->ConsumeTailAsStringViewUnsafe(len));
new_items.emplace_back(block->ConsumeTailAsStringViewUnsafe(len));
}

items_.swap(new_items);
blocks_.swap(new_blocks);

return true;
}

Expand Down

0 comments on commit 1f745e6

Please sign in to comment.