From 79f311f781310dff47ed0c6968d611243acc7802 Mon Sep 17 00:00:00 2001 From: wangwei <1261385937@qq.com> Date: Wed, 1 Nov 2023 16:13:03 +0800 Subject: [PATCH] code change for review --- clickhouse/columns/array.cpp | 3 +-- clickhouse/columns/decimal.cpp | 3 +-- clickhouse/columns/enum.cpp | 3 +-- clickhouse/columns/geo.cpp | 3 +-- clickhouse/columns/ip4.cpp | 3 +-- clickhouse/columns/ip6.cpp | 3 +-- clickhouse/columns/lowcardinality.cpp | 3 +-- clickhouse/columns/map.cpp | 3 +-- clickhouse/columns/nullable.cpp | 3 +-- clickhouse/columns/string.cpp | 17 +++++++++++------ clickhouse/columns/string.h | 2 +- clickhouse/columns/tuple.cpp | 7 ++++--- clickhouse/columns/uuid.cpp | 3 +-- 13 files changed, 26 insertions(+), 30 deletions(-) diff --git a/clickhouse/columns/array.cpp b/clickhouse/columns/array.cpp index cf088186..1867d778 100644 --- a/clickhouse/columns/array.cpp +++ b/clickhouse/columns/array.cpp @@ -52,8 +52,7 @@ ColumnRef ColumnArray::CloneEmpty() const { return std::make_shared(data_->CloneEmpty()); } -void ColumnArray::Reserve(size_t new_cap) -{ +void ColumnArray::Reserve(size_t new_cap) { data_->Reserve(new_cap); offsets_->Reserve(new_cap); } diff --git a/clickhouse/columns/decimal.cpp b/clickhouse/columns/decimal.cpp index 37de8647..2d214ecf 100644 --- a/clickhouse/columns/decimal.cpp +++ b/clickhouse/columns/decimal.cpp @@ -191,8 +191,7 @@ Int128 ColumnDecimal::At(size_t i) const { } } -void ColumnDecimal::Reserve(size_t new_cap) -{ +void ColumnDecimal::Reserve(size_t new_cap) { data_->Reserve(new_cap); } diff --git a/clickhouse/columns/enum.cpp b/clickhouse/columns/enum.cpp index c6729e64..43fab893 100644 --- a/clickhouse/columns/enum.cpp +++ b/clickhouse/columns/enum.cpp @@ -69,8 +69,7 @@ void ColumnEnum::SetNameAt(size_t n, const std::string& name) { } template -void ColumnEnum::Reserve(size_t new_cap) -{ +void ColumnEnum::Reserve(size_t new_cap) { data_.reserve(new_cap); } diff --git a/clickhouse/columns/geo.cpp b/clickhouse/columns/geo.cpp index b7bd2fc4..fa987732 100644 --- a/clickhouse/columns/geo.cpp +++ b/clickhouse/columns/geo.cpp @@ -55,8 +55,7 @@ const typename ColumnGeo::ValueType ColumnGeo -void ColumnGeo::Reserve(size_t new_cap) -{ +void ColumnGeo::Reserve(size_t new_cap) { data_->Reserve(new_cap); } diff --git a/clickhouse/columns/ip4.cpp b/clickhouse/columns/ip4.cpp index bc70ece6..8790afb6 100644 --- a/clickhouse/columns/ip4.cpp +++ b/clickhouse/columns/ip4.cpp @@ -74,8 +74,7 @@ std::string ColumnIPv4::AsString(size_t n) const { return ip_str; } -void ColumnIPv4::Reserve(size_t new_cap) -{ +void ColumnIPv4::Reserve(size_t new_cap) { data_->Reserve(new_cap); } diff --git a/clickhouse/columns/ip6.cpp b/clickhouse/columns/ip6.cpp index 1400ed01..0d47b5e8 100644 --- a/clickhouse/columns/ip6.cpp +++ b/clickhouse/columns/ip6.cpp @@ -65,8 +65,7 @@ in6_addr ColumnIPv6::operator [] (size_t n) const { return *reinterpret_cast(data_->At(n).data()); } -void ColumnIPv6::Reserve(size_t new_cap) -{ +void ColumnIPv6::Reserve(size_t new_cap) { data_->Reserve(new_cap); } diff --git a/clickhouse/columns/lowcardinality.cpp b/clickhouse/columns/lowcardinality.cpp index 95fce6a7..19369d33 100644 --- a/clickhouse/columns/lowcardinality.cpp +++ b/clickhouse/columns/lowcardinality.cpp @@ -174,8 +174,7 @@ ColumnLowCardinality::ColumnLowCardinality(std::shared_ptr dicti ColumnLowCardinality::~ColumnLowCardinality() {} -void ColumnLowCardinality::Reserve(size_t new_cap) -{ +void ColumnLowCardinality::Reserve(size_t new_cap) { dictionary_column_->Reserve(new_cap); index_column_->Reserve(new_cap); } diff --git a/clickhouse/columns/map.cpp b/clickhouse/columns/map.cpp index f98902c3..839b0668 100644 --- a/clickhouse/columns/map.cpp +++ b/clickhouse/columns/map.cpp @@ -33,8 +33,7 @@ ColumnMap::ColumnMap(ColumnRef data) : Column(GetMapType(data->GetType())), data_(data->As()) { } -void ColumnMap::Reserve(size_t new_cap) -{ +void ColumnMap::Reserve(size_t new_cap) { data_->Reserve(new_cap); } diff --git a/clickhouse/columns/nullable.cpp b/clickhouse/columns/nullable.cpp index d02b7f2c..23940c12 100644 --- a/clickhouse/columns/nullable.cpp +++ b/clickhouse/columns/nullable.cpp @@ -34,8 +34,7 @@ ColumnRef ColumnNullable::Nulls() const return nulls_; } -void ColumnNullable::Reserve(size_t new_cap) -{ +void ColumnNullable::Reserve(size_t new_cap) { nested_->Reserve(new_cap); nulls_->Reserve(new_cap); } diff --git a/clickhouse/columns/string.cpp b/clickhouse/columns/string.cpp index 07655967..791c2c6c 100644 --- a/clickhouse/columns/string.cpp +++ b/clickhouse/columns/string.cpp @@ -30,6 +30,10 @@ ColumnFixedString::ColumnFixedString(size_t n) { } +void ColumnFixedString::Reserve(size_t new_cap) { + data_.reserve(string_size_ * new_cap); +} + void ColumnFixedString::Append(std::string_view str) { if (str.size() > string_size_) { throw ValidationError("Expected string of length not greater than " @@ -45,8 +49,10 @@ void ColumnFixedString::Append(std::string_view str) { data_.insert(data_.size(), str); // Pad up to string_size_ with zeroes. - const auto padding_size = string_size_ - str.size(); - data_.resize(data_.size() + padding_size, char(0)); + if (str.size() < string_size_) { + const auto padding_size = string_size_ - str.size(); + data_.resize(data_.size() + padding_size, char(0)); + } } void ColumnFixedString::Clear() { @@ -160,8 +166,8 @@ ColumnString::ColumnString(size_t element_count) : Column(Type::CreateString()) { items_.reserve(element_count); - // 100 is arbitrary number, assumption that string values are about ~40 bytes long. - blocks_.reserve(std::max(1, element_count / 100)); + // 16 is arbitrary number, assumption that string values are about ~256 bytes long. + blocks_.reserve(std::max(1, element_count / 16)); } ColumnString::ColumnString(const std::vector& data) @@ -190,8 +196,7 @@ ColumnString::ColumnString(std::vector&& data) ColumnString::~ColumnString() {} -void ColumnString::Reserve(size_t new_cap) -{ +void ColumnString::Reserve(size_t new_cap) { items_.reserve(new_cap); // 16 is arbitrary number, assumption that string values are about ~256 bytes long. blocks_.reserve(std::max(1, new_cap / 16)); diff --git a/clickhouse/columns/string.h b/clickhouse/columns/string.h index 6d6d5e77..d6006556 100644 --- a/clickhouse/columns/string.h +++ b/clickhouse/columns/string.h @@ -28,7 +28,7 @@ class ColumnFixedString : public Column { } /// Increase the capacity of the column for large block insertion. - void Reserve(size_t) override {}; + void Reserve(size_t) override; /// Appends one element to the column. void Append(std::string_view str); diff --git a/clickhouse/columns/tuple.cpp b/clickhouse/columns/tuple.cpp index 24864063..56858590 100644 --- a/clickhouse/columns/tuple.cpp +++ b/clickhouse/columns/tuple.cpp @@ -20,9 +20,10 @@ size_t ColumnTuple::TupleSize() const { return columns_.size(); } -void ColumnTuple::Reserve(size_t new_cap) -{ - columns_.reserve(new_cap); +void ColumnTuple::Reserve(size_t new_cap) { + for (auto& column : columns_) { + column->Reserve(new_cap); + } } void ColumnTuple::Append(ColumnRef column) { diff --git a/clickhouse/columns/uuid.cpp b/clickhouse/columns/uuid.cpp index 72d3d2b8..fbaff97d 100644 --- a/clickhouse/columns/uuid.cpp +++ b/clickhouse/columns/uuid.cpp @@ -34,8 +34,7 @@ const UUID ColumnUUID::At(size_t n) const { return UUID(data_->At(n * 2), data_->At(n * 2 + 1)); } -void ColumnUUID::Reserve(size_t new_cap) -{ +void ColumnUUID::Reserve(size_t new_cap) { data_->Reserve(new_cap); }