Add Tuple field names and Bool Type#485
Conversation
This extends the type parser to support an element name alongside the element type and wires this through to make the field names in a tuple available in the C++ API.
Previously the library implicitly converted bool columns to `Uint8`, loosing type information. This commit introduces a "strong type" for `Bool` that is distinct from `bool`. This allows complete re-use of `ColumnVector` without triggering the `std::vector<bool>` specialization.
There was a problem hiding this comment.
Pull request overview
Adds first-class support for ClickHouse Bool (distinct from UInt8) and exposes Tuple field names through the type/column system to avoid extra schema queries when dealing with named tuples.
Changes:
- Introduce
clickhouse::BoolandType::Code::Bool, plusColumnBooland factory/type-parser support forBool. - Extend tuple parsing and type/column creation to retain tuple element (field) names (
Tuple(a UInt8, b String)), and keep them throughSlice()/CloneEmpty(). - Add/adjust unit tests and update README/.gitignore accordingly.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
clickhouse/types/types.h |
Adds Bool type and tuple item name plumbing in type APIs. |
clickhouse/types/types.cpp |
Wires Bool into type naming/IDs and adds named-tuple constructor/CreateTuple overload. |
clickhouse/types/type_parser.h |
Extends TypeAst to carry tuple element names. |
clickhouse/types/type_parser.cpp |
Parses named tuple elements and maps Bool to the new type code. |
clickhouse/columns/numeric.h / clickhouse/columns/numeric.cpp |
Adds ColumnBool and instantiates ColumnVector<Bool>. |
clickhouse/columns/factory.cpp |
Creates ColumnBool and builds ColumnTuple with names when present in the AST. |
clickhouse/columns/tuple.h / clickhouse/columns/tuple.cpp |
Adds named ColumnTuple constructor and preserves names across slicing/cloning. |
clickhouse/columns/itemview.h / clickhouse/columns/itemview.cpp |
Enables ItemView storage/access/validation for Bool. |
ut/types_ut.cpp |
Adds tests for Bool and tuple item-name retrieval (including from factory). |
ut/type_parser_ut.cpp |
Adds parser test for named tuples; adjusts AST expectations. |
ut/client_ut.cpp |
Updates integration-style client tests to use Bool columns/types. |
ut/CreateColumnByType_ut.cpp |
Updates factory test expectations for Bool. |
ut/utils.cpp |
Adds printing support for ItemView of Bool. |
README.md |
Documents Bool as supported type; trims trailing blank lines. |
.gitignore |
Ignores clangd cache directory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| TypeRef Type::CreateTuple(const std::vector<TypeRef>& item_types, | ||
| std::vector<std::string> item_names) { | ||
| return TypeRef(new TupleType(item_types, std::move(item_names))); | ||
| } |
There was a problem hiding this comment.
Named tuple field names are now stored (CreateTuple(..., item_names) / TupleType(item_names_)), but TupleType::GetName() currently omits those names. Since the native protocol writer sends bi.Type()->GetName() when inserting blocks, this means a named tuple parsed from something like Tuple(a UInt8, b String) will be serialized back as Tuple(UInt8, String), losing information and making type identity (GetTypeUniqueId / IsEqual) ignore item_names_. Consider updating tuple name rendering (and thus unique-id/equality) to incorporate item_names_ when present.
| && code == other.code | ||
| && name == other.name | ||
| && element_name == other.element_name | ||
| && value == other.value |
There was a problem hiding this comment.
TypeAst::operator== now compares element_name but still ignores value_string, so ASTs that differ only by string parameters (e.g. timezones, enum labels) will incorrectly compare equal. Include value_string in the equality check to make TypeAst comparisons accurate.
| && value == other.value | |
| && value == other.value | |
| && value_string == other.value_string |
|
@mshustov I thought about this a bit more over the evening and realized that the addition of I don't know what your policy is for such a potentially breaking change. I saw that 2.6.0 added new types, so I assume it would be fine. Still I think this deserves some consideration, given that |
This resolves two issues I just ran into when writing an integration to fetch data from ClickHouse.
Both changes are rather simple and self-contained in indivudual commits. Each adds a test for the newly added functionality.