Fix logical error with IPv4 in Protobuf, add support for Date32 #48486
Conversation
| @@ -3508,7 +3513,7 @@ namespace | |||
| } | |||
|
|
|||
| default: | |||
| throw Exception(ErrorCodes::LOGICAL_ERROR, "Unknown data type: {}", data_type->getName()); | |||
| throw Exception(ErrorCodes::ILLEGAL_COLUMN, "Type {} is not supported in Protobuf format", data_type->getName()); | |||
There was a problem hiding this comment.
Let's not throw LOGICAL_ERROR here. We can add new types in future and forget to support it in Protobuf format. It's ok to thow just ILLEGAL_COLUMN.
| } | ||
| }; | ||
|
|
||
| class ProtobufSerializerIPv4 : public ProtobufSerializerNumber<IPv4> |
There was a problem hiding this comment.
Could we write : public ProtobufSerializerNumber<UInt32> here for consistency with ProtobufSerializerDate?
There was a problem hiding this comment.
We cannot, because otherwise we will get logical error Bad cast from type DB::ColumnVector<StrongTypedef<unsigned int, DB::IPv4Tag>> to DB::ColumnVector<unsigned int>'. IPv4 type is not the same as UInt32
There was a problem hiding this comment.
Yeah, I see. DataTypeDate is derived from DataTypeNumberBase<UInt16> however for unknown reason DataTypeIPv4 is not derived from DataTypeNumberBase<UInt32>.
Probably if DataTypeIPv4 was derived from DataTypeNumberBase<UInt32> with ColumnType = ColumnVector<UInt32> , that would make things easier.
There was a problem hiding this comment.
I tried to change it to DataTypeNumberBase<UInt32> with ColumnType = ColumnVector<UInt32>, but seems like it won't work as in some places we need to distinguish between ColumnVector<UInt32> and ColumnIPv4 (for example for proper CAST I guess) and that's the reason it's implemented as separate DataType. Let's keep it as is.
robot-ch-test-poll2
added
the
pr-bugfix
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fix possible logical errors
Logical error: 'Unknown data type: Date32'andBad cast from type DB::ColumnVector<StrongTypedef<unsigned int, DB::IPv4Tag>> to DB::ColumnVector<unsigned int>'in Protobuf format.