-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat(bigtable): support update column family's value type to non-aggregate type #10410
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
Conversation
|
Add integration test |
| } | ||
|
|
||
| func (ac *AdminClient) setValueTypeImpl(ctx context.Context, table, family string, valueType Type) error { | ||
| if _, ok := valueType.proto().GetKind().(*btapb.Type_AggregateType); ok { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check valueType nil
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it needed? valueType is Type and not pointer to Type so it should always be non-null, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the test, i passed nil
err := c.SetValueType(context.Background(), "My-table", "cf1", nil)
if err != nil && !test.hasError {
t.Fatalf("Unexpected error when setting type: %v", err)
}
and I don't get any compile time error but see a panic when test is run:
--- FAIL: TestTableAdmin_SetType (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x10d7942]
goroutine 10 [running]:
testing.tRunner.func1.2({0x129d780, 0x2139e20})
/usr/lib/google-golang/src/testing/testing.go:1631 +0x24a
testing.tRunner.func1()
/usr/lib/google-golang/src/testing/testing.go:1634 +0x377
panic({0x129d780?, 0x2139e20?})
/usr/lib/google-golang/src/runtime/panic.go:759 +0x132
cloud.google.com/go/bigtable.(*AdminClient).setValueTypeImpl(0xc000138c80, {0x16876d0, 0x21a42c0}, {0x14e794e, 0x8}, {0x14e3557, 0x3}, {0x0, 0x0})
/usr/local/google/home/bahaaiman/Documents/cfdb-workspace-01/hoang/google-cloud-go/bigtable/admin.go:711 +0x62
cloud.google.com/go/bigtable.(*AdminClient).SetValueType(...)
/usr/local/google/home/bahaaiman/Documents/cfdb-workspace-01/hoang/google-cloud-go/bigtable/admin.go:739
cloud.google.com/go/bigtable.TestTableAdmin_SetType(0xc000124820)
/usr/local/google/home/bahaaiman/Documents/cfdb-workspace-01/hoang/google-cloud-go/bigtable/admin_test.go:433 +0x205
testing.tRunner(0xc000124820, 0x1556ea0)
/usr/lib/google-golang/src/testing/testing.go:1689 +0xfb
created by testing.(*T).Run in goroutine 1
/usr/lib/google-golang/src/testing/testing.go:1742 +0x390
FAIL cloud.google.com/go/bigtable 0.156s
FAIL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a nil check.
Done: |
…non-aggregate type (googleapis#10410)" This reverts commit df60464.
…non-aggregate type (googleapis#10410)" (googleapis#10576) This reverts commit df60464.
BEGIN_COMMIT_OVERRIDE
chore(bigtable): reverted Support update column family's value type to non-aggregate type
END_COMMIT_OVERRIDE