feat(Spanner): Add support for Protobuf Enums#15699
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for Spanner Enum types by adding a ProtobufEnumName property, a constructor, and helper methods to SpannerDbType, as well as supporting conversion of protobuf values to CLR enum types. The review feedback suggests correcting the string representation of the enum type from PROTO to ENUM, handling nullable enum types correctly during conversion, and fixing a typo in a parameter name.
| if (targetClrType.IsEnum) | ||
| { | ||
| return System.Enum.ToObject(targetClrType, ConvertToClrTypeImpl<long>(protobufValue, options)); | ||
| } |
There was a problem hiding this comment.
If targetClrType is a nullable enum (e.g., MyEnum?), targetClrType.IsEnum will evaluate to false. To ensure that nullable enum columns can be read correctly, we should resolve the underlying type using Nullable.GetUnderlyingType before checking if it is an enum.
System.Type enumType = Nullable.GetUnderlyingType(targetClrType) ?? targetClrType;
if (enumType.IsEnum)
{
return System.Enum.ToObject(enumType, ConvertToClrTypeImpl<long>(protobufValue, options));
}There was a problem hiding this comment.
targetClrType is already set to the underlying non-nullable type before we reach this line:
var possibleUnderlyingType = Nullable.GetUnderlyingType(targetClrType);
if (possibleUnderlyingType != null)
{
targetClrType = possibleUnderlyingType;
}
b/522532048