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.

Sign up for GitHub

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

Jump to bottom

fix(bigtable/bttest): emulator too lenient regarding RowFilter and CheckAndMutateRow conditions #4095

Merged
merged 5 commits into from Jun 3, 2021
Merged

fix(bigtable/bttest): emulator too lenient regarding RowFilter and CheckAndMutateRow conditions #4095

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e2f38ac
fix(bigtable/bttest): fix several test requests
dragonsinth May 12, 2021
b92936e
Merge branch 'master' into bttestfix
crwilcox Jun 2, 2021
c1dd801
Merge branch 'master' into bttestfix
crwilcox Jun 2, 2021
5443721
Merge branch 'master' into bttestfix
crwilcox Jun 3, 2021
57d049e
Merge branch 'master' into bttestfix
crwilcox Jun 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 14 additions & 2 deletions bigtable/bttest/inmem.go
View file
Open in desktop
Expand Up @@ -481,10 +481,19 @@ func filterRow(f *btpb.RowFilter, r *row) (bool, error) {
// Handle filters that apply beyond just including/excluding cells.
switch f := f.Filter.(type) {
case *btpb.RowFilter_BlockAllFilter:
return !f.BlockAllFilter, nil
if !f.BlockAllFilter {
return false, status.Errorf(codes.InvalidArgument, "block_all_filter must be true if set")
}
return false, nil
case *btpb.RowFilter_PassAllFilter:
return f.PassAllFilter, nil
if !f.PassAllFilter {
return false, status.Errorf(codes.InvalidArgument, "pass_all_filter must be true if set")
}
return true, nil
case *btpb.RowFilter_Chain_:
if len(f.Chain.Filters) < 2 {
return false, status.Errorf(codes.InvalidArgument, "Chain must contain at least two RowFilters")
}
for _, sub := range f.Chain.Filters {
match, err := filterRow(sub, r)
if err != nil {
Expand All @@ -496,6 +505,9 @@ func filterRow(f *btpb.RowFilter, r *row) (bool, error) {
}
return true, nil
case *btpb.RowFilter_Interleave_:
if len(f.Interleave.Filters) < 2 {
return false, status.Errorf(codes.InvalidArgument, "Interleave must contain at least two RowFilters")
}
srs := make([]*row, 0, len(f.Interleave.Filters))
for _, sub := range f.Interleave.Filters {
sr := r.copy()
Expand Down
72 changes: 52 additions & 20 deletions bigtable/bttest/inmem_test.go
View file
Open in desktop
Expand Up @@ -34,6 +34,8 @@ import (
btapb "google.golang.org/genproto/googleapis/bigtable/admin/v2"
btpb "google.golang.org/genproto/googleapis/bigtable/v2"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)

func TestConcurrentMutationsReadModifyAndGC(t *testing.T) {
Expand Down Expand Up @@ -833,27 +835,29 @@ func TestCheckAndMutateRowWithoutPredicate(t *testing.T) {
t.Fatalf("Creating table: %v", err)
}

// Populate the table
val := []byte("value")
muts := []*btpb.Mutation{{
Mutation: &btpb.Mutation_SetCell_{SetCell: &btpb.Mutation_SetCell{
FamilyName: "cf",
ColumnQualifier: []byte("col"),
TimestampMicros: 1000,
Value: val,
}},
}}

mrreq := &btpb.MutateRowRequest{
TableName: tbl.Name,
RowKey: []byte("row-present"),
Mutations: []*btpb.Mutation{{
Mutation: &btpb.Mutation_SetCell_{SetCell: &btpb.Mutation_SetCell{
FamilyName: "cf",
ColumnQualifier: []byte("col"),
TimestampMicros: 1000,
Value: val,
}},
}},
Mutations: muts,
}
if _, err := s.MutateRow(ctx, mrreq); err != nil {
t.Fatalf("Populating table: %v", err)
}

req := &btpb.CheckAndMutateRowRequest{
TableName: tbl.Name,
RowKey: []byte("row-not-present"),
TableName: tbl.Name,
RowKey: []byte("row-not-present"),
FalseMutations: muts,
}
if res, err := s.CheckAndMutateRow(ctx, req); err != nil {
t.Errorf("CheckAndMutateRow error: %v", err)
Expand All @@ -862,8 +866,9 @@ func TestCheckAndMutateRowWithoutPredicate(t *testing.T) {
}

req = &btpb.CheckAndMutateRowRequest{
TableName: tbl.Name,
RowKey: []byte("row-present"),
TableName: tbl.Name,
RowKey: []byte("row-present"),
FalseMutations: muts,
}
if res, err := s.CheckAndMutateRow(ctx, req); err != nil {
t.Errorf("CheckAndMutateRow error: %v", err)
Expand Down Expand Up @@ -923,6 +928,14 @@ func TestCheckAndMutateRowWithPredicate(t *testing.T) {
}
}

var bogusMutations = []*btpb.Mutation{{
Mutation: &btpb.Mutation_DeleteFromFamily_{
DeleteFromFamily: &btpb.Mutation_DeleteFromFamily{
FamilyName: "bogus_family",
},
},
}}

tests := []struct {
req *btpb.CheckAndMutateRowRequest
wantMatch bool
Expand All @@ -935,11 +948,13 @@ func TestCheckAndMutateRowWithPredicate(t *testing.T) {
{
req: &btpb.CheckAndMutateRowRequest{
TableName: tbl.Name,
RowKey: []byte("row1"),
PredicateFilter: &btpb.RowFilter{
Filter: &btpb.RowFilter_RowKeyRegexFilter{
RowKeyRegexFilter: []byte("not-one"),
},
},
TrueMutations: bogusMutations,
},
name: "no match",
},
Expand All @@ -952,6 +967,7 @@ func TestCheckAndMutateRowWithPredicate(t *testing.T) {
RowKeyRegexFilter: []byte("ro.+"),
},
},
FalseMutations: bogusMutations,
},
wantMatch: true,
name: "rowkey regex",
Expand All @@ -965,6 +981,7 @@ func TestCheckAndMutateRowWithPredicate(t *testing.T) {
PassAllFilter: true,
},
},
FalseMutations: bogusMutations,
},
wantMatch: true,
name: "pass all",
Expand Down Expand Up @@ -1274,13 +1291,14 @@ func populateTable(ctx context.Context, s *server) (*btapb.Table, error) {

func TestFilters(t *testing.T) {
tests := []struct {
in *btpb.RowFilter
out int
in *btpb.RowFilter
code codes.Code
out int
}{
{in: &btpb.RowFilter{Filter: &btpb.RowFilter_BlockAllFilter{true}}, out: 0},
{in: &btpb.RowFilter{Filter: &btpb.RowFilter_BlockAllFilter{false}}, out: 1},
{in: &btpb.RowFilter{Filter: &btpb.RowFilter_BlockAllFilter{false}}, code: codes.InvalidArgument},
{in: &btpb.RowFilter{Filter: &btpb.RowFilter_PassAllFilter{true}}, out: 1},
{in: &btpb.RowFilter{Filter: &btpb.RowFilter_PassAllFilter{false}}, out: 0},
{in: &btpb.RowFilter{Filter: &btpb.RowFilter_PassAllFilter{false}}, code: codes.InvalidArgument},
}

ctx := context.Background()
Expand All @@ -1303,7 +1321,16 @@ func TestFilters(t *testing.T) {
req.Filter = tc.in

mock := &MockReadRowsServer{}
if err = s.ReadRows(req, mock); err != nil {
err := s.ReadRows(req, mock)
if tc.code != codes.OK {
s, _ := status.FromError(err)
if s.Code() != tc.code {
t.Errorf("error code: got %d, want %d", s.Code(), tc.code)
}
continue
}

if err != nil {
t.Errorf("ReadRows error: %v", err)
continue
}
Expand Down Expand Up @@ -1672,14 +1699,19 @@ func TestFilterRowWithSingleColumnQualifier(t *testing.T) {
EndValue: &btpb.ValueRange_EndValueClosed{EndValueClosed: []byte("a")},
}},
},
{Filter: &btpb.RowFilter_PassAllFilter{PassAllFilter: true}},
}},
}},
TrueFilter: &btpb.RowFilter{Filter: &btpb.RowFilter_PassAllFilter{PassAllFilter: true}},
},
}}},
}},
{Filter: &btpb.RowFilter_BlockAllFilter{BlockAllFilter: true}},
},
},
},
}}},
},
{Filter: &btpb.RowFilter_PassAllFilter{PassAllFilter: true}},
}},
}},
}

Expand Down