Fix Nullable and LowCardinality serialization for set index #4594
Conversation
| @@ -47,7 +47,16 @@ void MergeTreeSetIndexGranule::serializeBinary(WriteBuffer & ostr) const | |||
| for (size_t i = 0; i < index.columns.size(); ++i) | |||
| { | |||
| const auto & type = index.data_types[i]; | |||
| type->serializeBinaryBulk(*columns[i], ostr, 0, size()); | |||
|
|
|||
| IDataType::SerializeBinaryBulkSettings settings; | |||
There was a problem hiding this comment.
Does it make sense to serialize DataTypeLowCardinality for index of type set as usual?
There was a problem hiding this comment.
Bulk serialization is needed for other types also. Nullable, for example.
For LowCardinality it doesn't (here we can only serialize dictionary, and even do not create set). But code will be a little bit more difficult.
Also, I don't understand why do we need to create set after deserialization.
There was a problem hiding this comment.
Also, I don't understand why do we need to create set after deserialization.
For quick filtering of granules?
There was a problem hiding this comment.
Also, I don't understand why do we need to create set after deserialization.
Yes, we do not need set there, a simple block will be fine. I'll fix it later by extracting aggregation logic into another class.
I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en
Category (leave one):
Short description (up to few sentences):
Fixed Set index for
NullableandLowCardinalitycolumns. Before it, set index withNullableorLowCardinalitycolumn led to errorData type must be deserialized with multiple streamswhile selecting.