Skip to content

Commit 005a21e

Browse files
authored
Merge pull request #309 from Enmk/fix_ColumnString_LoadBody
Updated ColumnString::LoadBody
2 parents 85dae8e + 1f745e6 commit 005a21e

File tree

1 file changed

+19
-8
lines changed

1 file changed

+19
-8
lines changed

clickhouse/columns/string.cpp

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -252,27 +252,38 @@ void ColumnString::Append(ColumnRef column) {
252252
}
253253

254254
bool ColumnString::LoadBody(InputStream* input, size_t rows) {
255-
items_.clear();
256-
blocks_.clear();
255+
if (rows == 0) {
256+
items_.clear();
257+
blocks_.clear();
258+
259+
return true;
260+
}
257261

258-
items_.reserve(rows);
259-
Block * block = nullptr;
262+
decltype(items_) new_items;
263+
decltype(blocks_) new_blocks;
264+
265+
new_items.reserve(rows);
266+
267+
// Suboptimzal if the first row string is >DEFAULT_BLOCK_SIZE, but that must be a very rare case.
268+
Block * block = &new_blocks.emplace_back(DEFAULT_BLOCK_SIZE);
260269

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

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

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

273-
items_.emplace_back(block->ConsumeTailAsStringViewUnsafe(len));
281+
new_items.emplace_back(block->ConsumeTailAsStringViewUnsafe(len));
274282
}
275283

284+
items_.swap(new_items);
285+
blocks_.swap(new_blocks);
286+
276287
return true;
277288
}
278289

0 commit comments

Comments
 (0)