Skip to content

feat(Spanner): Add support for Protobuf Enums#15699

Draft
efevans wants to merge 9 commits into
googleapis:mainfrom
efevans:spanner/protobuf-enums
Draft

feat(Spanner): Add support for Protobuf Enums#15699
efevans wants to merge 9 commits into
googleapis:mainfrom
efevans:spanner/protobuf-enums

Conversation

@efevans

@efevans efevans commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

b/522532048

@product-auto-label product-auto-label Bot added the api: spanner Issues related to the Spanner API. label Jun 22, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +113 to +116
if (targetClrType.IsEnum)
{
return System.Enum.ToObject(targetClrType, ConvertToClrTypeImpl<long>(protobufValue, options));
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

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));
            }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
            }

Comment thread apis/Google.Cloud.Spanner.Data/Google.Cloud.Spanner.Data/SpannerDbType.cs Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: spanner Issues related to the Spanner API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant