Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(bigtable): add column family type to FamilyInfo in TableInfo #10182

Closed
wants to merge 2 commits into from

Conversation

steveniemitz
Copy link
Contributor

This adds ValueType to FamilyInfo and populates it when calling TableInfo.

Internally it adds methods to convert from protobuf Type to our own type model.

…ponse

Change-Id: Iaa688f27c3472717929fbbdd03f3e12b8402c34a
@steveniemitz steveniemitz requested review from a team as code owners May 15, 2024 17:31
@product-auto-label product-auto-label bot added the api: bigtable Issues related to the Bigtable API. label May 15, 2024
@bhshkh
Copy link
Contributor

bhshkh commented May 16, 2024

Please add integration tests

it := protoToType(agg.InputType)

var aggregator Aggregator
switch agg.Aggregator.(type) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nil checks missing on agg and agg.Aggregator

}

func int64EncodingProtoToEncoding(ie *btapb.Type_Int64_Encoding) Int64Encoding {
switch e := ie.Encoding.(type) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nil checks missing on ie and ie.encoding

}

func (ua unknownAggregator) fillProto(proto *btapb.Type_Aggregate) {
proto.Aggregator = ua.wrapped.Aggregator
Copy link
Contributor

Choose a reason for hiding this comment

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

pointer nil check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

having basically no experience writing go, what am I supposed to do if its nil?

Copy link
Contributor

Choose a reason for hiding this comment

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

Please add nil checks wherever needed

@bhshkh
Copy link
Contributor

bhshkh commented Sep 5, 2024

This has been already added in #10520

Please reopen if something more needs to be added

@bhshkh bhshkh closed this Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigtable Issues related to the Bigtable API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants