Skip to content

Commit

Permalink
Revert "feat(bigtable): support update column family's value type to …
Browse files Browse the repository at this point in the history
…non-aggregate type (#10410)" (#10576)

This reverts commit df60464.
  • Loading branch information
bhshkh authored Jul 23, 2024
1 parent b1284bf commit 58ffd42
Show file tree
Hide file tree
Showing 3 changed files with 0 additions and 129 deletions.
31 changes: 0 additions & 31 deletions bigtable/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -688,9 +688,6 @@ func IgnoreWarnings() GCPolicyOption {
}

func (ac *AdminClient) setGCPolicy(ctx context.Context, table, family string, policy GCPolicy, opts ...GCPolicyOption) error {
if policy == nil {
return fmt.Errorf("policy cannot be nil")
}
ctx = mergeOutgoingMetadata(ctx, ac.md)
prefix := ac.instancePrefix()

Expand All @@ -712,41 +709,13 @@ func (ac *AdminClient) setGCPolicy(ctx context.Context, table, family string, po
return err
}

func (ac *AdminClient) setValueTypeImpl(ctx context.Context, table, family string, valueType Type) error {
if valueType == nil {
return fmt.Errorf("value type must be non nil")
}
if _, ok := valueType.proto().GetKind().(*btapb.Type_AggregateType); ok {
return fmt.Errorf("update family value type to aggregate type is unsupported")
}

ctx = mergeOutgoingMetadata(ctx, ac.md)
prefix := ac.instancePrefix()

req := &btapb.ModifyColumnFamiliesRequest{
Name: prefix + "/tables/" + table,
Modifications: []*btapb.ModifyColumnFamiliesRequest_Modification{{
Id: family,
Mod: &btapb.ModifyColumnFamiliesRequest_Modification_Update{Update: &btapb.ColumnFamily{ValueType: valueType.proto()}},
}},
}
_, err := ac.tClient.ModifyColumnFamilies(ctx, req)
return err
}

// SetGCPolicy specifies which cells in a column family should be garbage collected.
// GC executes opportunistically in the background; table reads may return data
// matching the GC policy.
func (ac *AdminClient) SetGCPolicy(ctx context.Context, table, family string, policy GCPolicy) error {
return ac.SetGCPolicyWithOptions(ctx, table, family, policy)
}

// SetValueType specifies the type of all values in a column family. Currently,
// only non-aggregate type is supported.
func (ac *AdminClient) SetValueType(ctx context.Context, table, family string, valueType Type) error {
return ac.setValueTypeImpl(ctx, table, family, valueType)
}

// SetGCPolicyWithOptions is similar to SetGCPolicy but allows passing options
func (ac *AdminClient) SetGCPolicyWithOptions(ctx context.Context, table, family string, policy GCPolicy, opts ...GCPolicyOption) error {
return ac.setGCPolicy(ctx, table, family, policy, opts...)
Expand Down
32 changes: 0 additions & 32 deletions bigtable/admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -410,38 +410,6 @@ func TestTableAdmin_SetGcPolicy(t *testing.T) {
}
}

func TestTableAdmin_SetType(t *testing.T) {
for _, test := range []struct {
desc string
familyType Type
hasError bool
}{
{
desc: "Update with aggregate type failed",
familyType: AggregateType{Input: Int64Type{Encoding: BigEndianBytesEncoding{}}, Aggregator: SumAggregator{}},
hasError: true,
},
{
desc: "Update with string type",
familyType: StringType{Encoding: StringUtf8Encoding{}},
hasError: false,
},
{
desc: "Update with nil type",
familyType: nil,
hasError: true,
},
} {
mock := &mockTableAdminClock{}
c := setupTableClient(t, mock)

err := c.SetValueType(context.Background(), "My-table", "cf1", test.familyType)
if err != nil && !test.hasError {
t.Fatalf("Unexpected error when setting type: %v", err)
}
}
}

func TestTableAdmin_CreateAuthorizedView_DeletionProtection_Protected(t *testing.T) {
mock := &mockTableAdminClock{}
c := setupTableClient(t, mock)
Expand Down
66 changes: 0 additions & 66 deletions bigtable/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3977,72 +3977,6 @@ func TestIntegration_DataAuthorizedView(t *testing.T) {
}
}

func TestIntegration_TestUpdateColumnFamilyValueType(t *testing.T) {
testEnv, err := NewIntegrationEnv()
if err != nil {
t.Fatalf("IntegrationEnv: %v", err)
}
defer testEnv.Close()

if !testEnv.Config().UseProd {
t.Skip("emulator doesn't support update column family operation")
}

timeout := 15 * time.Minute
ctx, cancel := context.WithTimeout(context.Background(), timeout)
defer cancel()

adminClient, err := testEnv.NewAdminClient()
if err != nil {
t.Fatalf("NewAdminClient: %v", err)
}
defer adminClient.Close()

tblConf := TableConf{
TableID: testEnv.Config().Table,
ColumnFamilies: map[string]Family{
"cf": {
GCPolicy: MaxVersionsPolicy(1),
},
},
}
if err := adminClient.CreateTableFromConf(ctx, &tblConf); err != nil {
t.Fatalf("Create table from TableConf error: %v", err)
}
// Clean-up
defer deleteTable(ctx, t, adminClient, tblConf.TableID)

// Update column family type to aggregate type should not be successful
err = adminClient.SetValueType(ctx, tblConf.TableID, "cf", AggregateType{
Input: Int64Type{}, Aggregator: SumAggregator{},
})
if err == nil {
t.Fatalf("Update family type to aggregate type should not be successful")
}

// Update column family type to string type should be successful
err = adminClient.SetValueType(ctx, tblConf.TableID, "cf", StringType{
Encoding: StringUtf8Encoding{},
})
if err != nil {
t.Fatalf("Failed to update value type of family: %v", err)
}

table, err := adminClient.TableInfo(ctx, tblConf.TableID)
if err != nil {
t.Fatalf("Failed to get table info: %v", err)
}
if len(table.FamilyInfos) != 0 {
t.Fatalf("Unexpected number of family infos. Got %d, want %d", len(table.FamilyInfos), 0)
}
if table.FamilyInfos[0].Name != "cf" {
t.Errorf("Unexpected family name. Got %q, want %q", table.FamilyInfos[0].Name, "cf")
}
if _, ok := table.FamilyInfos[0].ValueType.proto().GetKind().(*btapb.Type_StringType); !ok {
t.Errorf("Unexpected value type. Got %T, want *btapb.Type_StringType", table.FamilyInfos[0].ValueType.proto().GetKind())
}
}

// TestIntegration_DirectPathFallback tests the CFE fallback when the directpath net is blackholed.
func TestIntegration_DirectPathFallback(t *testing.T) {
ctx := context.Background()
Expand Down

0 comments on commit 58ffd42

Please sign in to comment.