From e854748650442b528da49f9b9929c382a07de863 Mon Sep 17 00:00:00 2001 From: gerrra Date: Sat, 27 Jan 2024 03:25:15 +0300 Subject: [PATCH 1/3] Added `GetSize()` const method for FixedStringType By using ColumnLowCardinalityT, you lose the ability to conveniently know the fixed size parameter of ColumnFixedString. Adding this method fixes this issue. --- clickhouse/types/types.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/clickhouse/types/types.h b/clickhouse/types/types.h index 423a6f70..12e2cbd5 100644 --- a/clickhouse/types/types.h +++ b/clickhouse/types/types.h @@ -251,6 +251,8 @@ class FixedStringType : public Type { std::string GetName() const { return std::string("FixedString(") + std::to_string(size_) + ")"; } + inline size_t GetSize() const { return size_; } + private: size_t size_; }; From 677ade6786b1bce7024884695cb0022e140c2560 Mon Sep 17 00:00:00 2001 From: gerrra Date: Sun, 28 Jan 2024 18:35:06 +0300 Subject: [PATCH 2/3] Added unit-tests for `FixedStringType::GetSize()` method to columns_ut.cpp --- ut/columns_ut.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/ut/columns_ut.cpp b/ut/columns_ut.cpp index aaad5f26..439c5d92 100644 --- a/ut/columns_ut.cpp +++ b/ut/columns_ut.cpp @@ -104,6 +104,16 @@ TEST(ColumnsCase, FixedString_Append_LargeString) { EXPECT_ANY_THROW(col->Append("this is a long string")); } +TEST(ColumnsCase, FixedString_Type_Size_Eq0) { + const auto col = std::make_shared(0); + ASSERT_EQ(col->FixedSize(), col->Type()->As()->GetSize()); +} + +TEST(ColumnsCase, FixedString_Type_Size_Eq10) { + const auto col = std::make_shared(10); + ASSERT_EQ(col->FixedSize(), col->Type()->As()->GetSize()); +} + TEST(ColumnsCase, StringInit) { auto values = MakeStrings(); auto col = std::make_shared(values); @@ -866,6 +876,12 @@ TEST(ColumnsCase, ColumnLowCardinalityString_WithEmptyString_3) { } } +TEST(ColumnsCase, ColumnLowCardinalityFixedString_Type_Size_Eq) { + const auto fixed_size = 10; + const auto col = std::make_shared>(fixed_size); + + ASSERT_EQ(fixed_size, col->GetNestedType()->As()->GetSize()); +} TEST(ColumnsCase, ColumnTupleT) { using TestTuple = ColumnTupleT; From a7ac900e255866eebae9595031c7d23d9bf48185 Mon Sep 17 00:00:00 2001 From: gerrra Date: Mon, 29 Jan 2024 16:09:49 +0300 Subject: [PATCH 3/3] Use size_t instead of auto in unit-test (LowCardinalityFixedString). --- ut/columns_ut.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ut/columns_ut.cpp b/ut/columns_ut.cpp index 439c5d92..623fdf0e 100644 --- a/ut/columns_ut.cpp +++ b/ut/columns_ut.cpp @@ -877,8 +877,8 @@ TEST(ColumnsCase, ColumnLowCardinalityString_WithEmptyString_3) { } TEST(ColumnsCase, ColumnLowCardinalityFixedString_Type_Size_Eq) { - const auto fixed_size = 10; - const auto col = std::make_shared>(fixed_size); + const size_t fixed_size = 10; + const auto col = std::make_shared>(fixed_size); ASSERT_EQ(fixed_size, col->GetNestedType()->As()->GetSize()); }