diff --git a/go/test/endtoend/keyspace/keyspace_test.go b/go/test/endtoend/keyspace/keyspace_test.go index 82477dacee7..c8ff519910f 100644 --- a/go/test/endtoend/keyspace/keyspace_test.go +++ b/go/test/endtoend/keyspace/keyspace_test.go @@ -17,7 +17,6 @@ limitations under the License. package sequence import ( - "bytes" "encoding/binary" "encoding/json" "flag" @@ -25,6 +24,8 @@ import ( "strings" "testing" + "vitess.io/vitess/go/vt/key" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -392,8 +393,8 @@ func TestKeyspaceToShardName(t *testing.T) { shardKIDs := shardKIdMap[shardRef.Name] for _, kid := range shardKIDs { id = packKeyspaceID(kid) - assert.True(t, bytes.Compare(shardRef.KeyRange.Start, id) <= 0 && - (len(shardRef.KeyRange.End) == 0 || bytes.Compare(id, shardRef.KeyRange.End) < 0)) + assert.True(t, key.Compare(shardRef.KeyRange.Start, id) <= 0 && + (key.Empty(shardRef.KeyRange.End) || key.Compare(id, shardRef.KeyRange.End) < 0)) } } } diff --git a/go/vt/discovery/topology_watcher.go b/go/vt/discovery/topology_watcher.go index 5e5c54ab57a..8a4dcb3213e 100644 --- a/go/vt/discovery/topology_watcher.go +++ b/go/vt/discovery/topology_watcher.go @@ -362,7 +362,7 @@ func (fbs *FilterByShard) IsIncluded(tablet *topodata.Tablet) bool { // Exact match (probably a non-sharded keyspace). return true } - if kr != nil && c.keyRange != nil && key.KeyRangeIncludes(c.keyRange, kr) { + if kr != nil && c.keyRange != nil && key.KeyRangeContainsKeyRange(c.keyRange, kr) { // Our filter's KeyRange includes the provided KeyRange return true } diff --git a/go/vt/key/destination.go b/go/vt/key/destination.go index c4fb37b7a9d..437e980f480 100644 --- a/go/vt/key/destination.go +++ b/go/vt/key/destination.go @@ -128,7 +128,7 @@ func (d DestinationExactKeyRange) String() string { func processExactKeyRange(allShards []*topodatapb.ShardReference, kr *topodatapb.KeyRange, addShard func(shard string) error) error { sort.SliceStable(allShards, func(i, j int) bool { - return KeyRangeStartSmaller(allShards[i].GetKeyRange(), allShards[j].GetKeyRange()) + return KeyRangeLess(allShards[i].GetKeyRange(), allShards[j].GetKeyRange()) }) shardnum := 0 @@ -139,7 +139,7 @@ func processExactKeyRange(allShards []*topodatapb.ShardReference, kr *topodatapb shardnum++ } for shardnum < len(allShards) { - if !KeyRangesIntersect(kr, allShards[shardnum].KeyRange) { + if !KeyRangeIntersect(kr, allShards[shardnum].KeyRange) { // If we are over the requested keyrange, we // can stop now, we won't find more. break @@ -215,7 +215,7 @@ func (d DestinationKeyRange) String() string { func processKeyRange(allShards []*topodatapb.ShardReference, kr *topodatapb.KeyRange, addShard func(shard string) error) error { for _, shard := range allShards { - if !KeyRangesIntersect(kr, shard.KeyRange) { + if !KeyRangeIntersect(kr, shard.KeyRange) { // We don't need that shard. continue } diff --git a/go/vt/key/key.go b/go/vt/key/key.go index fc603554ecf..dcdcda47f81 100644 --- a/go/vt/key/key.go +++ b/go/vt/key/key.go @@ -26,16 +26,18 @@ import ( "regexp" "strings" - "google.golang.org/protobuf/proto" - topodatapb "vitess.io/vitess/go/vt/proto/topodata" ) +var ( + KeyRangePattern = regexp.MustCompile(`^(0|([0-9a-fA-F]{2})*-([0-9a-fA-F]{2})*)$`) +) + // // Uint64Key definitions // -// Uint64Key is a uint64 that can be converted into a KeyspaceId. +// Uint64Key is a uint64 that can be converted into a keyspace id. type Uint64Key uint64 func (i Uint64Key) String() string { @@ -49,261 +51,214 @@ func (i Uint64Key) Bytes() []byte { return buf } -// -// KeyRange helper methods -// +// Helper methods for keyspace id values. -// EvenShardsKeyRange returns a key range definition for a shard at index "i", -// assuming range based sharding with "n" equal-width shards in total. -// i starts at 0. -// -// Example: (1, 2) returns the second out of two shards in total i.e. "80-". -// -// This function must not be used in the Vitess code base because Vitess also -// supports shards with different widths. In that case, the output of this -// function would be wrong. -// -// Note: start and end values have trailing zero bytes omitted. -// For example, "80-" has only the first byte (0x80) set. -// We do this to produce the same KeyRange objects as ParseKeyRangeParts() does. -// Because it's using the Go hex methods, it's omitting trailing zero bytes as -// well. -func EvenShardsKeyRange(i, n int) (*topodatapb.KeyRange, error) { - if n <= 0 { - return nil, fmt.Errorf("the shard count must be > 0: %v", n) - } - if i >= n { - return nil, fmt.Errorf("the index of the shard must be less than the total number of shards: %v < %v", i, n) - } - if n&(n-1) != 0 { - return nil, fmt.Errorf("the shard count must be a power of two: %v", n) +// Normalize removes any trailing zero bytes from id. This allows two id values to be compared even if they are +// different lengths. +// From a key range perspective, -80 == 00-80 == 0000-8000 == 000000-800000, etc. and they should +// always be treated the same even if they are different lengths. +func Normalize(id []byte) []byte { + trailingZeroes := 0 + for i := len(id) - 1; i >= 0 && id[i] == 0x00; i-- { + trailingZeroes += 1 } - // Determine the number of bytes which are required to represent any - // KeyRange start or end for the given n. - // This is required to trim the returned values to the same length e.g. - // (256, 512) should return 8000-8080 as shard key range. - minBytes := 0 - for nn := Uint64Key(n - 1); nn > 0; nn >>= 8 { - minBytes++ - } + return id[:len(id)-trailingZeroes] +} - width := Uint64Key(math.MaxUint64)/Uint64Key(n) + 1 - start := Uint64Key(i) * width - end := start + width +// Compare compares two keyspace IDs while taking care to normalize them; returns -1 if ab, 0 if equal. +func Compare(a, b []byte) int { + return bytes.Compare(Normalize(a), Normalize(b)) +} - // Note: The byte value is empty if start or end is the min or the max - // respectively. - startBytes := start.Bytes()[:minBytes] - endBytes := end.Bytes()[:minBytes] - if start == 0 { - startBytes = []byte{} - } - if end == 0 { - // Always set the end except for the last shard. In that case, the - // end value (2^64) flows over and is the same as 0. - endBytes = []byte{} - } - return &topodatapb.KeyRange{Start: startBytes, End: endBytes}, nil +// Less returns true if a is less than b. +func Less(a, b []byte) bool { + return Compare(a, b) < 0 +} + +// Equal returns true if a is equal to b. +func Equal(a, b []byte) bool { + return Compare(a, b) == 0 +} + +// Empty returns true if id is an empty keyspace ID. +func Empty(id []byte) bool { + return len(Normalize(id)) == 0 } -// KeyRangeAdd adds two adjacent keyranges into a single value. -// If the values are not adjacent, it returns false. -func KeyRangeAdd(first, second *topodatapb.KeyRange) (*topodatapb.KeyRange, bool) { - if first == nil || second == nil { +// +// KeyRange helper methods +// + +// KeyRangeAdd adds two adjacent KeyRange values (in any order) into a single value. If the values are not adjacent, +// it returns false. +func KeyRangeAdd(a, b *topodatapb.KeyRange) (*topodatapb.KeyRange, bool) { + if a == nil || b == nil { return nil, false } - if len(first.End) != 0 && bytes.Equal(first.End, second.Start) { - return &topodatapb.KeyRange{Start: first.Start, End: second.End}, true + if !Empty(a.End) && Equal(a.End, b.Start) { + return &topodatapb.KeyRange{Start: a.Start, End: b.End}, true } - if len(second.End) != 0 && bytes.Equal(second.End, first.Start) { - return &topodatapb.KeyRange{Start: second.Start, End: first.End}, true + if !Empty(b.End) && Equal(b.End, a.Start) { + return &topodatapb.KeyRange{Start: b.Start, End: a.End}, true } return nil, false } // KeyRangeContains returns true if the provided id is in the keyrange. -func KeyRangeContains(kr *topodatapb.KeyRange, id []byte) bool { - if kr == nil { +func KeyRangeContains(keyRange *topodatapb.KeyRange, id []byte) bool { + if KeyRangeIsComplete(keyRange) { return true } - return bytes.Compare(kr.Start, id) <= 0 && - (len(kr.End) == 0 || bytes.Compare(id, kr.End) < 0) + return (Empty(keyRange.Start) || Compare(id, keyRange.Start) >= 0) && (Empty(keyRange.End) || Compare(id, keyRange.End) < 0) } -// ParseKeyRangeParts parses a start and end hex values and build a proto KeyRange +// ParseKeyRangeParts parses a Start and End as hex encoded strings and builds a proto KeyRange. func ParseKeyRangeParts(start, end string) (*topodatapb.KeyRange, error) { - s, err := hex.DecodeString(start) + startKey, err := hex.DecodeString(start) if err != nil { return nil, err } - e, err := hex.DecodeString(end) + endKey, err := hex.DecodeString(end) if err != nil { return nil, err } - return &topodatapb.KeyRange{Start: s, End: e}, nil + return &topodatapb.KeyRange{Start: startKey, End: endKey}, nil } -// KeyRangeString prints a topodatapb.KeyRange -func KeyRangeString(k *topodatapb.KeyRange) string { - if k == nil { +// KeyRangeString formats a topodatapb.KeyRange into a hex encoded string. +func KeyRangeString(keyRange *topodatapb.KeyRange) string { + if KeyRangeIsComplete(keyRange) { return "-" } - return hex.EncodeToString(k.Start) + "-" + hex.EncodeToString(k.End) + return hex.EncodeToString(keyRange.Start) + "-" + hex.EncodeToString(keyRange.End) } -// KeyRangeIsPartial returns true if the KeyRange does not cover the entire space. -func KeyRangeIsPartial(kr *topodatapb.KeyRange) bool { - if kr == nil { - return false +// KeyRangeStartCompare compares the Start of two KeyRange values using semantics unique to Start values where an +// empty Start means the *minimum* value; returns -1 if ab, 0 if equal. +func KeyRangeStartCompare(a, b *topodatapb.KeyRange) int { + aIsMinimum := a == nil || Empty(a.Start) + bIsMinimum := b == nil || Empty(b.Start) + + if aIsMinimum && bIsMinimum { + return 0 + } else if aIsMinimum { + return -1 + } else if bIsMinimum { + return 1 } - return !(len(kr.Start) == 0 && len(kr.End) == 0) + + return Compare(a.Start, b.Start) } -// KeyRangeEqual returns true if both key ranges cover the same area -func KeyRangeEqual(left, right *topodatapb.KeyRange) bool { - if left == nil { - return right == nil || (len(right.Start) == 0 && len(right.End) == 0) - } - if right == nil { - return len(left.Start) == 0 && len(left.End) == 0 - } - return bytes.Equal(addPadding(left.Start), addPadding(right.Start)) && - bytes.Equal(addPadding(left.End), addPadding(right.End)) +// KeyRangeStartEqual returns true if both KeyRange values have the same Start. +func KeyRangeStartEqual(a, b *topodatapb.KeyRange) bool { + return KeyRangeStartCompare(a, b) == 0 } -// addPadding adds padding to make sure keyrange represents an 8 byte integer. -// From Vitess docs: -// A hash vindex produces an 8-byte number. -// This means that all numbers less than 0x8000000000000000 will fall in shard -80. -// Any number with the highest bit set will be >= 0x8000000000000000, and will therefore -// belong to shard 80-. -// This means that from a keyrange perspective -80 == 00-80 == 0000-8000 == 000000-800000 -// If we don't add this padding, we could run into issues when transitioning from keyranges -// that use 2 bytes to 4 bytes. -func addPadding(kr []byte) []byte { - paddedKr := make([]byte, 8) - - for i := 0; i < len(kr); i++ { - paddedKr = append(paddedKr, kr[i]) - } +// KeyRangeEndCompare compares the End of two KeyRange values using semantics unique to End values where an +// empty End means the *maximum* value; returns -1 if ab, 0 if equal. +func KeyRangeEndCompare(a, b *topodatapb.KeyRange) int { + aIsMaximum := a == nil || Empty(a.End) + bIsMaximum := b == nil || Empty(b.End) - for i := len(kr); i < 8; i++ { - paddedKr = append(paddedKr, 0) + if aIsMaximum && bIsMaximum { + return 0 + } else if aIsMaximum { + return 1 + } else if bIsMaximum { + return -1 } - return paddedKr + + return Compare(a.End, b.End) } -// KeyRangeStartSmaller returns true if right's keyrange start is _after_ left's start -func KeyRangeStartSmaller(left, right *topodatapb.KeyRange) bool { - if left == nil { - return right != nil - } - if right == nil { - return false - } - return bytes.Compare(left.Start, right.Start) < 0 +// KeyRangeEndEqual returns true if both KeyRange values have the same End. +func KeyRangeEndEqual(a, b *topodatapb.KeyRange) bool { + return KeyRangeEndCompare(a, b) == 0 } -// KeyRangeStartEqual returns true if both key ranges have the same start -func KeyRangeStartEqual(left, right *topodatapb.KeyRange) bool { - if left == nil { - return right == nil || len(right.Start) == 0 - } - if right == nil { - return len(left.Start) == 0 - } - return bytes.Equal(addPadding(left.Start), addPadding(right.Start)) +// KeyRangeCompare compares two KeyRange values, taking into account both the Start and End fields and their +// field-specific comparison logic; returns -1 if ab, 0 if equal. Specifically: +// +// - The Start-specific KeyRangeStartCompare and End-specific KeyRangeEndCompare are used for proper comparison +// of an empty value for either Start or End. +// - The Start is compared first and End is only compared if Start is equal. +func KeyRangeCompare(a, b *topodatapb.KeyRange) int { + // First, compare the Start field. + if v := KeyRangeStartCompare(a, b); v != 0 { + // The Start field for a and b differ, and that is enough; return that comparison. + return v + } + + // The Start field was equal, so compare the End field and return that comparison. + return KeyRangeEndCompare(a, b) } -// KeyRangeContiguous returns true if the end of the left key range exactly -// matches the start of the right key range (i.e they are contigious) -func KeyRangeContiguous(left, right *topodatapb.KeyRange) bool { - if left == nil { - return right == nil || (len(right.Start) == 0 && len(right.End) == 0) - } - if right == nil { - return len(left.Start) == 0 && len(left.End) == 0 - } - return bytes.Equal(addPadding(left.End), addPadding(right.Start)) +// KeyRangeEqual returns true if a and b are equal. +func KeyRangeEqual(a, b *topodatapb.KeyRange) bool { + return KeyRangeCompare(a, b) == 0 } -// KeyRangeEndEqual returns true if both key ranges have the same end -func KeyRangeEndEqual(left, right *topodatapb.KeyRange) bool { - if left == nil { - return right == nil || len(right.End) == 0 - } - if right == nil { - return len(left.End) == 0 - } - return bytes.Equal(addPadding(left.End), addPadding(right.End)) +// KeyRangeLess returns true if a is less than b. +func KeyRangeLess(a, b *topodatapb.KeyRange) bool { + return KeyRangeCompare(a, b) < 0 } -// For more info on the following functions, see: -// See: http://stackoverflow.com/questions/4879315/what-is-a-tidy-algorithm-to-find-overlapping-intervals -// two segments defined as (a,b) and (c,d) (with a c) && (a < d) -// overlap = min(b, d) - max(c, a) - -// KeyRangesIntersect returns true if some Keyspace values exist in both ranges. -func KeyRangesIntersect(first, second *topodatapb.KeyRange) bool { - if first == nil || second == nil { - return true - } - return (len(first.End) == 0 || bytes.Compare(second.Start, first.End) < 0) && - (len(second.End) == 0 || bytes.Compare(first.Start, second.End) < 0) +// KeyRangeIsComplete returns true if the KeyRange covers the entire keyspace. +func KeyRangeIsComplete(keyRange *topodatapb.KeyRange) bool { + return keyRange == nil || (Empty(keyRange.Start) && Empty(keyRange.End)) } -// KeyRangesOverlap returns the overlap between two KeyRanges. -// They need to overlap, otherwise an error is returned. -func KeyRangesOverlap(first, second *topodatapb.KeyRange) (*topodatapb.KeyRange, error) { - if !KeyRangesIntersect(first, second) { - return nil, fmt.Errorf("KeyRanges %v and %v don't overlap", first, second) - } - if first == nil { - return second, nil - } - if second == nil { - return first, nil - } - // compute max(c,a) and min(b,d) - // start with (a,b) - result := proto.Clone(first).(*topodatapb.KeyRange) - // if c > a, then use c - if bytes.Compare(second.Start, first.Start) > 0 { - result.Start = second.Start +// KeyRangeIsPartial returns true if the KeyRange does not cover the entire keyspace. +func KeyRangeIsPartial(keyRange *topodatapb.KeyRange) bool { + return !KeyRangeIsComplete(keyRange) +} + +// KeyRangeContiguous returns true if the End of KeyRange a is equivalent to the Start of the KeyRange b, +// which means that they are contiguous. +func KeyRangeContiguous(a, b *topodatapb.KeyRange) bool { + if KeyRangeIsComplete(a) || KeyRangeIsComplete(b) { + return false // no two KeyRange values can be contiguous if either is the complete range } - // if b is maxed out, or - // (d is not maxed out and d < b) - // ^ valid test as neither b nor d are max - // then use d - if len(first.End) == 0 || (len(second.End) != 0 && bytes.Compare(second.End, first.End) < 0) { - result.End = second.End + + return Equal(a.End, b.Start) +} + +// For more info on the following functions, see: +// http://stackoverflow.com/questions/4879315/what-is-a-tidy-algorithm-to-find-overlapping-intervals +// Two segments defined as (a,b) and (c,d) (with a c) && (a < d) +// * overlap = min(b, d) - max(c, a) + +// KeyRangeIntersect returns true if some part of KeyRange a and b overlap, meaning that some keyspace ID values +// exist in both a and b. +func KeyRangeIntersect(a, b *topodatapb.KeyRange) bool { + if KeyRangeIsComplete(a) || KeyRangeIsComplete(b) { + return true // if either KeyRange is complete, there must be an intersection } - return result, nil + + return (Empty(a.End) || Less(b.Start, a.End)) && (Empty(b.End) || Less(a.Start, b.End)) } -// KeyRangeIncludes returns true if the first provided KeyRange, big, -// contains the second KeyRange, small. If they intersect, but small -// spills out, this returns false. -func KeyRangeIncludes(big, small *topodatapb.KeyRange) bool { - if big == nil { - // The outside one covers everything, we're good. +// KeyRangeContainsKeyRange returns true if KeyRange a fully contains KeyRange b. +func KeyRangeContainsKeyRange(a, b *topodatapb.KeyRange) bool { + // If a covers the entire KeyRange, it always contains b. + if KeyRangeIsComplete(a) { return true } - if small == nil { - // The smaller one covers everything, better have the - // bigger one also cover everything. - return len(big.Start) == 0 && len(big.End) == 0 - } - // Now we check small.Start >= big.Start, and small.End <= big.End - if len(big.Start) != 0 && bytes.Compare(small.Start, big.Start) < 0 { - return false + + // If b covers the entire KeyRange, a must also cover the entire KeyRange. + if KeyRangeIsComplete(b) { + return KeyRangeIsComplete(a) } - if len(big.End) != 0 && (len(small.End) == 0 || bytes.Compare(small.End, big.End) > 0) { - return false + + // Ensure b.Start >= a.Start and b.End <= a.End. + if KeyRangeStartCompare(b, a) >= 0 && KeyRangeEndCompare(b, a) <= 0 { + return true } - return true + + return false } // ParseShardingSpec parses a string that describes a sharding @@ -351,11 +306,63 @@ func ParseShardingSpec(spec string) ([]*topodatapb.KeyRange, error) { return ranges, nil } -var krRegexp = regexp.MustCompile(`^[0-9a-fA-F]*-[0-9a-fA-F]*$`) +// IsValidKeyRange returns true if the string represents a valid key range. +func IsValidKeyRange(keyRangeString string) bool { + return KeyRangePattern.MatchString(keyRangeString) +} -// IsKeyRange returns true if the string represents a keyrange. -func IsKeyRange(kr string) bool { - return krRegexp.MatchString(kr) +// EvenShardsKeyRange returns a key range definition for a shard at index "i", +// assuming range based sharding with "n" equal-width shards in total. +// i starts at 0. +// +// Example: (1, 2) returns the second out of two shards in total i.e. "80-". +// +// This function must not be used in the Vitess code base because Vitess also +// supports shards with different widths. In that case, the output of this +// function would be wrong. +// +// Note: start and end values have trailing zero bytes omitted. +// For example, "80-" has only the first byte (0x80) set. +// We do this to produce the same KeyRange objects as ParseKeyRangeParts() does. +// Because it's using the Go hex methods, it's omitting trailing zero bytes as +// well. +func EvenShardsKeyRange(i, n int) (*topodatapb.KeyRange, error) { + if n <= 0 { + return nil, fmt.Errorf("the shard count must be > 0: %v", n) + } + if i >= n { + return nil, fmt.Errorf("the index of the shard must be less than the total number of shards: %v < %v", i, n) + } + if n&(n-1) != 0 { + return nil, fmt.Errorf("the shard count must be a power of two: %v", n) + } + + // Determine the number of bytes which are required to represent any + // KeyRange start or end for the given n. + // This is required to trim the returned values to the same length e.g. + // (256, 512) should return 8000-8080 as shard key range. + minBytes := 0 + for nn := Uint64Key(n - 1); nn > 0; nn >>= 8 { + minBytes++ + } + + width := Uint64Key(math.MaxUint64)/Uint64Key(n) + 1 + start := Uint64Key(i) * width + end := start + width + + // Note: The byte value is empty if start or end is the min or the max + // respectively. + startBytes := start.Bytes()[:minBytes] + endBytes := end.Bytes()[:minBytes] + if start == 0 { + startBytes = []byte{} + } + if end == 0 { + // Always set the end except for the last shard. In that case, the + // end value (2^64) flows over and is the same as 0. + endBytes = []byte{} + } + return &topodatapb.KeyRange{Start: startBytes, End: endBytes}, nil } // GenerateShardRanges returns shard ranges assuming a keyspace with N shards. diff --git a/go/vt/key/key_test.go b/go/vt/key/key_test.go index 639b81c5f18..8db45aa79b9 100644 --- a/go/vt/key/key_test.go +++ b/go/vt/key/key_test.go @@ -28,7 +28,222 @@ import ( topodatapb "vitess.io/vitess/go/vt/proto/topodata" ) -func TestKey(t *testing.T) { +func TestNormalize(t *testing.T) { + type args struct { + id []byte + } + tests := []struct { + name string + args args + want []byte + }{ + { + "empty should empty", + args{[]byte{}}, + []byte{}, + }, + { + "one zero should be empty", + args{[]byte{0x00}}, + []byte{}, + }, + { + "any number of zeroes should be empty", + args{[]byte{0x00, 0x00, 0x00}}, + []byte{}, + }, + { + "one non-zero byte should be left alone", + args{[]byte{0x11}}, + []byte{0x11}, + }, + { + "multiple non-zero bytes should be left alone", + args{[]byte{0x11, 0x22, 0x33}}, + []byte{0x11, 0x22, 0x33}, + }, + { + "zeroes that aren't trailing should be left alone", + args{[]byte{0x11, 0x00, 0x22, 0x00, 0x33, 0x00}}, + []byte{0x11, 0x00, 0x22, 0x00, 0x33}, + }, + { + "excess zero bytes should be removed after a non-zero byte", + args{[]byte{0x11, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}}, + []byte{0x11}, + }, + { + "excess zero bytes should be removed after multiple non-zero bytes", + args{[]byte{0x11, 0x22, 0x00, 0x00, 0x00}}, + []byte{0x11, 0x22}, + }, + { + "values longer than eight bytes should be supported", + args{[]byte{0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xaa, 0x00}}, + []byte{0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xaa}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equalf(t, tt.want, Normalize(tt.args.id), "Normalize(%v)", tt.args.id) + }) + } +} + +func TestCompare(t *testing.T) { + type args struct { + a []byte + b []byte + } + tests := []struct { + name string + args args + want int + }{ + { + "empty ids are equal", + args{[]byte{}, []byte{}}, + 0, + }, + { + "equal full id values are equal", + args{ + []byte{0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88}, + []byte{0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88}, + }, + 0, + }, + { + "equal partial id values are equal", + args{ + []byte{0x11, 0x22}, + []byte{0x11, 0x22}, + }, + 0, + }, + { + "equal full and partial id values are equal", + args{[]byte{0x11, 0x22, 0x33, 0x44}, []byte{0x11, 0x22, 0x33, 0x44, 0x00, 0x00, 0x00, 0x00}}, + 0, + }, + { + "equal partial and full id values are equal", + args{[]byte{0x11, 0x22, 0x33, 0x44, 0x00, 0x00, 0x00, 0x00}, []byte{0x11, 0x22, 0x33, 0x44}}, + 0, + }, + {"a less than b", args{[]byte{0x01}, []byte{0x02}}, -1}, + {"a greater than b", args{[]byte{0x02}, []byte{0x01}}, +1}, + { + "equal partial a and b with different lengths", + args{[]byte{0x30, 0x00}, []byte{0x20}}, + 1, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equalf(t, tt.want, Compare(tt.args.a, tt.args.b), "Compare(%v, %v)", tt.args.a, tt.args.b) + }) + } +} + +func TestLess(t *testing.T) { + type args struct { + a []byte + b []byte + } + tests := []struct { + name string + args args + want bool + }{ + // Less uses Compare which is already robustly tested, so we're just aiming to ensure that the result + // of the Compare is used correctly in context and not e.g. reversed, so test a few obvious cases. + { + "a is less than b", + args{[]byte{0x01}, []byte{0x02}}, + true, + }, + { + "a is equal to b", + args{[]byte{0x01}, []byte{0x01}}, + false, + }, + { + "a is greater than b", + args{[]byte{0x02}, []byte{0x01}}, + false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equalf(t, tt.want, Less(tt.args.a, tt.args.b), "Less(%v, %v)", tt.args.a, tt.args.b) + }) + } +} + +func TestEqual(t *testing.T) { + type args struct { + a []byte + b []byte + } + tests := []struct { + name string + args args + want bool + }{ + // Equal uses Compare which is already robustly tested, so we're just aiming to ensure that the result + // of the Compare is used correctly in context and not e.g. reversed, so test a few obvious cases. + { + "a is less than b", + args{[]byte{0x01}, []byte{0x02}}, + false, + }, + { + "a is equal to b", + args{[]byte{0x01}, []byte{0x01}}, + true, + }, + { + "a is greater than b", + args{[]byte{0x02}, []byte{0x01}}, + false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equalf(t, tt.want, Equal(tt.args.a, tt.args.b), "Equal(%v, %v)", tt.args.a, tt.args.b) + }) + } +} + +func TestEmpty(t *testing.T) { + type args struct { + id []byte + } + tests := []struct { + name string + args args + want bool + }{ + { + "empty", + args{[]byte{}}, + true, + }, + { + "not empty", + args{[]byte{0x11, 0x22, 0x33}}, + false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equalf(t, tt.want, Empty(tt.args.id), "Empty(%v)", tt.args.id) + }) + } +} + +func TestUint64Key(t *testing.T) { k0 := Uint64Key(0) k1 := Uint64Key(1) k2 := Uint64Key(0x7FFFFFFFFFFFFFFF) @@ -362,7 +577,7 @@ func TestKeyRangeContiguous(t *testing.T) { }, { first: "-", second: "-40", - out: true, + out: false, }, { first: "40-80", second: "c0-", @@ -460,7 +675,398 @@ func TestParseShardingSpec(t *testing.T) { } } -func TestContains(t *testing.T) { +func TestKeyRangeComparisons(t *testing.T) { + type args struct { + a *topodatapb.KeyRange + b *topodatapb.KeyRange + } + type wants struct { + wantStartCompare int + wantStartEqual bool + wantEndCompare int + wantEndEqual bool + wantCompare int + wantEqual bool + } + tests := []struct { + name string + args args + wants wants + }{ + { + name: "a and b are both full range", + args: args{ + a: stringToKeyRange("-"), + b: stringToKeyRange("-"), + }, + wants: wants{ + wantStartCompare: 0, + wantStartEqual: true, + wantEndCompare: 0, + wantEndEqual: true, + wantCompare: 0, + wantEqual: true, + }, + }, + { + name: "a is equal to b", + args: args{ + a: stringToKeyRange("10-30"), + b: stringToKeyRange("10-30"), + }, + wants: wants{ + wantStartCompare: 0, + wantStartEqual: true, + wantEndCompare: 0, + wantEndEqual: true, + wantCompare: 0, + wantEqual: true, + }, + }, + { + name: "a (2 digit, end only) but equal to b (2 digits, end only)", + args: args{ + a: stringToKeyRange("-80"), + b: stringToKeyRange("-80"), + }, + wants: wants{ + wantStartCompare: 0, + wantStartEqual: true, + wantEndCompare: 0, + wantEndEqual: true, + wantCompare: 0, + wantEqual: true, + }, + }, + { + name: "a (2 digit, end only) but equal to b (4 digits, end only)", + args: args{ + a: stringToKeyRange("-80"), + b: stringToKeyRange("-8000"), + }, + wants: wants{ + wantStartCompare: 0, + wantStartEqual: true, + wantEndCompare: 0, + wantEndEqual: true, + wantCompare: 0, + wantEqual: true, + }, + }, + { + name: "a (2 digit, end only) but equal to b (6 digits, end only)", + args: args{ + a: stringToKeyRange("-80"), + b: stringToKeyRange("-800000"), + }, + wants: wants{ + wantStartCompare: 0, + wantStartEqual: true, + wantEndCompare: 0, + wantEndEqual: true, + wantCompare: 0, + wantEqual: true, + }, + }, + { + name: "a (2 digit, end only) but equal to b (8 digits, end only)", + args: args{ + a: stringToKeyRange("-80"), + b: stringToKeyRange("-80000000"), + }, + wants: wants{ + wantStartCompare: 0, + wantStartEqual: true, + wantEndCompare: 0, + wantEndEqual: true, + wantCompare: 0, + wantEqual: true, + }, + }, + { + name: "a (2 digit, start only) but equal to b (2 digits, start only)", + args: args{ + stringToKeyRange("80-"), + stringToKeyRange("80-"), + }, + wants: wants{ + wantStartCompare: 0, + wantStartEqual: true, + wantEndCompare: 0, + wantEndEqual: true, + wantCompare: 0, + wantEqual: true, + }, + }, + { + name: "a (2 digit, start only) but equal to b (4 digits, start only)", + args: args{ + a: stringToKeyRange("80-"), + b: stringToKeyRange("8000-"), + }, + wants: wants{ + wantStartCompare: 0, + wantStartEqual: true, + wantEndCompare: 0, + wantEndEqual: true, + wantCompare: 0, + wantEqual: true, + }, + }, + { + name: "a (2 digit, start only) but equal to b (6 digits, start only)", + args: args{ + a: stringToKeyRange("80-"), + b: stringToKeyRange("800000-"), + }, + wants: wants{ + wantStartCompare: 0, + wantStartEqual: true, + wantEndCompare: 0, + wantEndEqual: true, + wantCompare: 0, + wantEqual: true, + }, + }, + { + name: "a (2 digit, start only) but equal to b (8 digits, start only)", + args: args{ + a: stringToKeyRange("80-"), + b: stringToKeyRange("80000000-"), + }, + wants: wants{ + wantStartCompare: 0, + wantStartEqual: true, + wantEndCompare: 0, + wantEndEqual: true, + wantCompare: 0, + wantEqual: true, + }, + }, + { + name: "a (4 digits) but equal to b (2 digits)", + args: args{ + a: stringToKeyRange("1000-3000"), + b: stringToKeyRange("10-30"), + }, + wants: wants{ + wantStartCompare: 0, + wantStartEqual: true, + wantEndCompare: 0, + wantEndEqual: true, + wantCompare: 0, + wantEqual: true, + }, + }, + { + name: "a (8 digits) but equal to b (4 digits)", + args: args{ + a: stringToKeyRange("10000000-30000000"), + b: stringToKeyRange("1000-3000"), + }, + wants: wants{ + wantStartCompare: 0, + wantStartEqual: true, + wantEndCompare: 0, + wantEndEqual: true, + wantCompare: 0, + wantEqual: true, + }, + }, + { + name: "b (4 digits) but equal to a (2 digits)", + args: args{ + a: stringToKeyRange("10-30"), + b: stringToKeyRange("1000-3000"), + }, + wants: wants{ + wantStartCompare: 0, + wantStartEqual: true, + wantEndCompare: 0, + wantEndEqual: true, + wantCompare: 0, + wantEqual: true, + }, + }, + { + name: "b (8 digits) but equal to a (4 digits)", + args: args{ + a: stringToKeyRange("10-30"), + b: stringToKeyRange("10000000-30000000"), + }, + wants: wants{ + wantStartCompare: 0, + wantStartEqual: true, + wantEndCompare: 0, + wantEndEqual: true, + wantCompare: 0, + wantEqual: true, + }, + }, + { + name: "a is full range, b is not", + args: args{ + a: stringToKeyRange("-"), + b: stringToKeyRange("20-30"), + }, + wants: wants{ + wantStartCompare: -1, + wantStartEqual: false, + wantEndCompare: 1, + wantEndEqual: false, + wantCompare: -1, + wantEqual: false, + }, + }, + { + name: "b is full range, a is not", + args: args{ + a: stringToKeyRange("10-30"), + b: stringToKeyRange("-"), + }, + wants: wants{ + wantStartCompare: 1, + wantStartEqual: false, + wantEndCompare: -1, + wantEndEqual: false, + wantCompare: 1, + wantEqual: false, + }, + }, + { + name: "a start is greater than b start", + args: args{ + a: stringToKeyRange("10-30"), + b: stringToKeyRange("20-30"), + }, + wants: wants{ + wantStartCompare: -1, + wantStartEqual: false, + wantEndCompare: 0, + wantEndEqual: true, + wantCompare: -1, + wantEqual: false, + }, + }, + { + name: "b start is greater than a start", + args: args{ + a: stringToKeyRange("20-30"), + b: stringToKeyRange("10-30"), + }, + wants: wants{ + wantStartCompare: 1, + wantStartEqual: false, + wantEndCompare: 0, + wantEndEqual: true, + wantCompare: 1, + wantEqual: false, + }, + }, + { + name: "a start is empty, b start is not", + args: args{ + a: stringToKeyRange("-30"), + b: stringToKeyRange("10-30"), + }, + wants: wants{ + wantStartCompare: -1, + wantStartEqual: false, + wantEndCompare: 0, + wantEndEqual: true, + wantCompare: -1, + wantEqual: false, + }, + }, + { + name: "b start is empty, a start is not", + args: args{ + a: stringToKeyRange("10-30"), + b: stringToKeyRange("-30"), + }, + wants: wants{ + wantStartCompare: 1, + wantStartEqual: false, + wantEndCompare: 0, + wantEndEqual: true, + wantCompare: 1, + wantEqual: false, + }, + }, + { + name: "a end is greater than b end", + args: args{ + a: stringToKeyRange("10-30"), + b: stringToKeyRange("10-20"), + }, + wants: wants{ + wantStartCompare: 0, + wantStartEqual: true, + wantEndCompare: 1, + wantEndEqual: false, + wantCompare: 1, + wantEqual: false, + }, + }, + { + name: "b end is greater than a end", + args: args{ + a: stringToKeyRange("10-20"), + b: stringToKeyRange("10-30"), + }, + wants: wants{ + wantStartCompare: 0, + wantStartEqual: true, + wantEndCompare: -1, + wantEndEqual: false, + wantCompare: -1, + wantEqual: false, + }, + }, + { + name: "a end is empty, b end is not", + args: args{ + a: stringToKeyRange("10-"), + b: stringToKeyRange("10-30"), + }, + wants: wants{ + wantStartCompare: 0, + wantStartEqual: true, + wantEndCompare: 1, + wantEndEqual: false, + wantCompare: 1, + wantEqual: false, + }, + }, + { + name: "b end is empty, a end is not", + args: args{ + a: stringToKeyRange("10-30"), + b: stringToKeyRange("10-"), + }, + wants: wants{ + wantStartCompare: 0, + wantStartEqual: true, + wantEndCompare: -1, + wantEndEqual: false, + wantCompare: -1, + wantEqual: false, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equalf(t, tt.wants.wantStartCompare, KeyRangeStartCompare(tt.args.a, tt.args.b), "KeyRangeStartCompare(%v, %v)", tt.args.a, tt.args.b) + assert.Equalf(t, tt.wants.wantStartEqual, KeyRangeStartEqual(tt.args.a, tt.args.b), "KeyRangeStartEqual(%v, %v)", tt.args.a, tt.args.b) + assert.Equalf(t, tt.wants.wantEndCompare, KeyRangeEndCompare(tt.args.a, tt.args.b), "KeyRangeEndCompare(%v, %v)", tt.args.a, tt.args.b) + assert.Equalf(t, tt.wants.wantEndEqual, KeyRangeEndEqual(tt.args.a, tt.args.b), "KeyRangeEndEqual(%v, %v)", tt.args.a, tt.args.b) + assert.Equalf(t, tt.wants.wantCompare, KeyRangeCompare(tt.args.a, tt.args.b), "KeyRangeCompare(%v, %v)", tt.args.a, tt.args.b) + assert.Equalf(t, tt.wants.wantEqual, KeyRangeEqual(tt.args.a, tt.args.b), "KeyRangeEqual(%v, %v)", tt.args.a, tt.args.b) + }) + } +} + +func TestKeyRangeContains(t *testing.T) { var table = []struct { kid string start string @@ -499,120 +1105,321 @@ func TestContains(t *testing.T) { } } -func TestIntersectOverlap(t *testing.T) { - var table = []struct { - a string - b string - c string - d string - intersects bool - overlap string - }{ - {a: "40", b: "80", c: "c0", d: "d0", intersects: false}, - {a: "", b: "80", c: "80", d: "", intersects: false}, - {a: "", b: "80", c: "", d: "40", intersects: true, overlap: "-40"}, - {a: "80", b: "", c: "c0", d: "", intersects: true, overlap: "c0-"}, - {a: "", b: "80", c: "40", d: "80", intersects: true, overlap: "40-80"}, - {a: "40", b: "80", c: "60", d: "a0", intersects: true, overlap: "60-80"}, - {a: "40", b: "80", c: "50", d: "60", intersects: true, overlap: "50-60"}, - {a: "40", b: "80", c: "10", d: "50", intersects: true, overlap: "40-50"}, - {a: "40", b: "80", c: "40", d: "80", intersects: true, overlap: "40-80"}, - {a: "", b: "80", c: "", d: "80", intersects: true, overlap: "-80"}, - {a: "40", b: "", c: "40", d: "", intersects: true, overlap: "40-"}, - {a: "40", b: "80", c: "20", d: "40", intersects: false}, - {a: "80", b: "", c: "80", d: "c0", intersects: true, overlap: "80-c0"}, - {a: "", b: "", c: "c0", d: "d0", intersects: true, overlap: "c0-d0"}, +func TestKeyRangeIntersect(t *testing.T) { + type args struct { + a *topodatapb.KeyRange + b *topodatapb.KeyRange } + tests := []struct { + name string + args args + want bool + }{ + // non-intersecting cases + { + name: "typical half-range split, ascending order", + args: args{a: stringToKeyRange("-80"), b: stringToKeyRange("80-")}, + want: false, + }, + { + name: "typical half-range split, descending order", + args: args{a: stringToKeyRange("80-"), b: stringToKeyRange("-80")}, + want: false, + }, + { + name: "partial ranges, ascending order", + args: args{a: stringToKeyRange("40-80"), b: stringToKeyRange("c0-d0")}, + want: false, + }, + { + name: "partial ranges, descending order", + args: args{a: stringToKeyRange("40-80"), b: stringToKeyRange("20-40")}, + want: false, + }, + { + name: "partial ranges, different key lengths", + args: args{a: stringToKeyRange("4000-8000"), b: stringToKeyRange("20-40")}, + want: false, + }, - for _, el := range table { - a, err := hex.DecodeString(el.a) - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - b, err := hex.DecodeString(el.b) - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - left := &topodatapb.KeyRange{Start: a, End: b} - c, err := hex.DecodeString(el.c) - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - d, err := hex.DecodeString(el.d) - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - right := &topodatapb.KeyRange{Start: c, End: d} - if c := KeyRangesIntersect(left, right); c != el.intersects { - t.Errorf("Unexpected result: KeyRangesIntersect for %v and %v yields %v.", left, right, c) - } - overlap, err := KeyRangesOverlap(left, right) - if el.intersects { - if err != nil { - t.Errorf("Unexpected result: KeyRangesOverlap for overlapping %v and %v returned an error: %v", left, right, err) - } else { - got := hex.EncodeToString(overlap.Start) + "-" + hex.EncodeToString(overlap.End) - if got != el.overlap { - t.Errorf("Unexpected result: KeyRangesOverlap for overlapping %v and %v should have returned: %v but got: %v", left, right, el.overlap, got) - } - } - } else { - if err == nil { - t.Errorf("Unexpected result: KeyRangesOverlap for non-overlapping %v and %v should have returned an error", left, right) - } - } + // intersecting cases with a full range + { + name: "full range with full range", + args: args{a: stringToKeyRange("-"), b: stringToKeyRange("-")}, + want: true, + }, + { + name: "full range with maximum key partial range", + args: args{a: stringToKeyRange("-"), b: stringToKeyRange("80-")}, + want: true, + }, + { + name: "full range with partial range", + args: args{a: stringToKeyRange("-"), b: stringToKeyRange("c0-d0")}, + want: true, + }, + { + name: "minimum key partial range with full range", + args: args{a: stringToKeyRange("-80"), b: stringToKeyRange("-")}, + want: true, + }, + { + name: "partial range with full range", + args: args{a: stringToKeyRange("a0-b0"), b: stringToKeyRange("-")}, + want: true, + }, + + // intersecting cases with only partial ranges + { + name: "the same range, both from minimum key", + args: args{a: stringToKeyRange("-80"), b: stringToKeyRange("-80")}, + want: true, + }, + { + name: "the same range, both from minimum key, different key lengths", + args: args{a: stringToKeyRange("-8000"), b: stringToKeyRange("-80")}, + want: true, + }, + { + name: "the same range, both to maximum key", + args: args{a: stringToKeyRange("40-"), b: stringToKeyRange("40-")}, + want: true, + }, + { + name: "the same range, both to maximum key, different key lengths", + args: args{a: stringToKeyRange("4000-"), b: stringToKeyRange("40-")}, + want: true, + }, + { + name: "the same range, both with mid-range keys", + args: args{a: stringToKeyRange("40-80"), b: stringToKeyRange("40-80")}, + want: true, + }, + { + name: "the same range, both with mid-range keys, different key lengths", + args: args{a: stringToKeyRange("40-80"), b: stringToKeyRange("4000-8000")}, + want: true, + }, + { + name: "different-sized partial ranges, both from minimum key", + args: args{a: stringToKeyRange("-80"), b: stringToKeyRange("-40")}, + want: true, + }, + { + name: "different-sized partial ranges, both to maximum key", + args: args{a: stringToKeyRange("80-"), b: stringToKeyRange("c0-")}, + want: true, + }, + { + name: "different-sized partial ranges, from minimum key with mid-range key", + args: args{a: stringToKeyRange("-80"), b: stringToKeyRange("40-80")}, + want: true, + }, + { + name: "different-sized partial ranges, from minimum key with mid-range key, different key lengths", + args: args{a: stringToKeyRange("-80"), b: stringToKeyRange("4000-8000")}, + want: true, + }, + { + name: "different-sized partial ranges, to maximum key with mid-range key", + args: args{a: stringToKeyRange("80-"), b: stringToKeyRange("80-c0")}, + want: true, + }, + { + name: "different-sized partial ranges, to maximum key with mid-range key, different key lengths", + args: args{a: stringToKeyRange("80-"), b: stringToKeyRange("8000-c000")}, + want: true, + }, + { + name: "partially overlapping ranges, in ascending order", + args: args{a: stringToKeyRange("40-80"), b: stringToKeyRange("60-a0")}, + want: true, + }, + { + name: "partially overlapping ranges, in descending order", + args: args{a: stringToKeyRange("40-80"), b: stringToKeyRange("10-50")}, + want: true, + }, + { + name: "partially overlapping ranges, one fully containing the other, in ascending order", + args: args{a: stringToKeyRange("40-80"), b: stringToKeyRange("50-60")}, + want: true, + }, + { + name: "partially overlapping ranges, one fully containing the other, in descending order", + args: args{a: stringToKeyRange("40-80"), b: stringToKeyRange("30-90")}, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equalf(t, tt.want, KeyRangeIntersect(tt.args.a, tt.args.b), "KeyRangeIntersect(%v, %v)", tt.args.a, tt.args.b) + }) } } -func TestKeyRangeIncludes(t *testing.T) { - var table = []struct { - name string - big string - small string - expected bool +func TestKeyRangeContainsKeyRange(t *testing.T) { + type args struct { + a *topodatapb.KeyRange + b *topodatapb.KeyRange + } + var tests = []struct { + name string + args args + want bool }{ - {"big nil, small nil", "nil", "nil", true}, - {"big nil, small non nil, fully partial", "nil", "80-c0", true}, - {"big nil, small non nil, full start", "nil", "-c0", true}, - {"big nil, small non nil, full end", "nil", "80-", true}, - {"big non-nil, fully partial, small nil", "80-c0", "nil", false}, - {"big non-nil, full start, small nil", "-c0", "nil", false}, - {"big non-nil, full end, small nil", "80-", "nil", false}, - {"big full, small full", "-", "-", true}, - {"big full, small partial", "-", "40-60", true}, - {"big partial, small full", "40-60", "-", false}, - - {"big partial, small to the end", "40-60", "40-", false}, - {"big partial, small bigger to the right", "40-60", "40-80", false}, - {"big partial, small equal", "40-60", "40-60", true}, - {"big partial, small smaller right", "40-60", "40-50", true}, - - {"big partial, small to the beginning", "40-60", "-60", false}, - {"big partial, small smaller to the left", "40-60", "20-60", false}, - {"big partial, small bigger left", "40-60", "50-60", true}, - } - - var err error - for _, tc := range table { - var big, small *topodatapb.KeyRange - if tc.big != "nil" { - parts := strings.Split(tc.big, "-") - big, err = ParseKeyRangeParts(parts[0], parts[1]) - if err != nil { - t.Fatalf("test data error in %v: %v", tc.big, err) - } - } - if tc.small != "nil" { - parts := strings.Split(tc.small, "-") - small, err = ParseKeyRangeParts(parts[0], parts[1]) - if err != nil { - t.Fatalf("test data error in %v: %v", tc.small, err) - } - } - got := KeyRangeIncludes(big, small) - if got != tc.expected { - t.Errorf("KeyRangeIncludes for test case '%v' returned %v but expected %v", tc.name, got, tc.expected) - } + // full range contains itself + { + name: "both full range", + args: args{a: stringToKeyRange("-"), b: stringToKeyRange("-")}, + want: true, + }, + + // full range always contains a partial range + { + name: "full range, partial range from minimum key", + args: args{a: stringToKeyRange("-"), b: stringToKeyRange("-c0")}, + want: true, + }, + { + name: "full range, partial range to maximum key", + args: args{a: stringToKeyRange("-"), b: stringToKeyRange("80-")}, + want: true, + }, + { + name: "full range, partial mid-key range", + args: args{a: stringToKeyRange("-"), b: stringToKeyRange("80-c0")}, + want: true, + }, + + // equal partial ranges contain each other + { + name: "equal partial ranges", + args: args{a: stringToKeyRange("40-60"), b: stringToKeyRange("40-60")}, + want: true, + }, + { + name: "equal partial ranges, different size keys", + args: args{a: stringToKeyRange("40-60"), b: stringToKeyRange("4000-6000")}, + want: true, + }, + { + name: "equal partial ranges, different size keys", + args: args{a: stringToKeyRange("4000-6000"), b: stringToKeyRange("40-60")}, + want: true, + }, + + // partial ranges may contain smaller partial ranges + { + name: "partial range, partial touching start", + args: args{a: stringToKeyRange("40-80"), b: stringToKeyRange("40-50")}, + want: true, + }, + { + name: "partial range, partial touching start, different size keys", + args: args{a: stringToKeyRange("40-80"), b: stringToKeyRange("4000-5000")}, + want: true, + }, + { + name: "partial range, partial touching start, different size keys", + args: args{a: stringToKeyRange("4000-8000"), b: stringToKeyRange("40-50")}, + want: true, + }, + { + name: "partial range, partial touching end", + args: args{a: stringToKeyRange("40-80"), b: stringToKeyRange("70-80")}, + want: true, + }, + { + name: "partial range, partial touching end, different size keys", + args: args{a: stringToKeyRange("40-80"), b: stringToKeyRange("7000-8000")}, + want: true, + }, + { + name: "partial range, partial touching end, different size keys", + args: args{a: stringToKeyRange("4000-8000"), b: stringToKeyRange("70-80")}, + want: true, + }, + { + name: "partial range, partial in the middle", + args: args{a: stringToKeyRange("40-80"), b: stringToKeyRange("50-70")}, + want: true, + }, + { + name: "partial range, partial in the middle, different size keys", + args: args{a: stringToKeyRange("40-80"), b: stringToKeyRange("5000-7000")}, + want: true, + }, + { + name: "partial range, partial in the middle, different size keys", + args: args{a: stringToKeyRange("4000-8000"), b: stringToKeyRange("50-70")}, + want: true, + }, + + // partial ranges do not contain the full range + { + name: "partial range from minimum key, full range", + args: args{a: stringToKeyRange("-c0"), b: stringToKeyRange("-")}, + want: false, + }, + { + name: "partial range to maximum key, full range", + args: args{a: stringToKeyRange("80-"), b: stringToKeyRange("-")}, + want: false, + }, + { + name: "partial mid-key range, full range", + args: args{a: stringToKeyRange("80-c0"), b: stringToKeyRange("-")}, + want: false, + }, + + // partial ranges do not contain overlapping but boundary-crossing partial ranges + { + name: "partial range mid-key range, overlapping partial range to maximum key", + args: args{a: stringToKeyRange("40-60"), b: stringToKeyRange("50-")}, + want: false, + }, + { + name: "partial range mid-key range, overlapping partial range to maximum key", + args: args{a: stringToKeyRange("40-60"), b: stringToKeyRange("5000-")}, + want: false, + }, + { + name: "partial range mid-key range, overlapping partial range to maximum key, different size keys", + args: args{a: stringToKeyRange("4000-6000"), b: stringToKeyRange("50-")}, + want: false, + }, + { + name: "partial range mid-key range, overlapping partial range to maximum key, different size keys", + args: args{a: stringToKeyRange("40-60"), b: stringToKeyRange("5000-")}, + want: false, + }, + { + name: "partial range mid-key range, overlapping partial range from minimum key", + args: args{a: stringToKeyRange("40-60"), b: stringToKeyRange("-50")}, + want: false, + }, + { + name: "partial range mid-key range, overlapping partial range from minimum key", + args: args{a: stringToKeyRange("40-60"), b: stringToKeyRange("-5000")}, + want: false, + }, + { + name: "partial range mid-key range, overlapping partial range from minimum key, different size keys", + args: args{a: stringToKeyRange("4000-6000"), b: stringToKeyRange("-50")}, + want: false, + }, + { + name: "partial range mid-key range, overlapping partial range from minimum key, different size keys", + args: args{a: stringToKeyRange("40-60"), b: stringToKeyRange("-5000")}, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equalf(t, tt.want, KeyRangeContainsKeyRange(tt.args.a, tt.args.b), "KeyRangeContainsKeyRange(%v, %v)", tt.args.a, tt.args.b) + }) } } @@ -671,65 +1478,40 @@ func BenchmarkKeyRangesIntersect(b *testing.B) { } for i := 0; i < b.N; i++ { - KeyRangesIntersect(kr1, kr2) + KeyRangeIntersect(kr1, kr2) } } -func BenchmarkKeyRangesOverlap(b *testing.B) { - kr1 := &topodatapb.KeyRange{ - Start: []byte{0x40, 0, 0, 0, 0, 0, 0, 0}, - End: []byte{0x80, 0, 0, 0, 0, 0, 0, 0}, - } - kr2 := &topodatapb.KeyRange{ - Start: []byte{0x30, 0, 0, 0, 0, 0, 0, 0}, - End: []byte{0x50, 0, 0, 0, 0, 0, 0, 0}, - } - - for i := 0; i < b.N; i++ { - if _, err := KeyRangesOverlap(kr1, kr2); err != nil { - b.Fatal(err) - } - } -} +func TestIsValidKeyRange(t *testing.T) { + tests := []struct { + arg string + want bool + }{ + // normal cases + {"-", true}, + {"00-", true}, + {"-80", true}, + {"40-80", true}, + {"80-", true}, + {"a0-", true}, + {"-A0", true}, -func TestIsKeyRange(t *testing.T) { - testcases := []struct { - in string - out bool - }{{ - in: "-", - out: true, - }, { - in: "-80", - out: true, - }, { - in: "40-80", - out: true, - }, { - in: "80-", - out: true, - }, { - in: "a0-", - out: true, - }, { - in: "-A0", - out: true, - }, { - in: "", - out: false, - }, { - in: "x-80", - out: false, - }, { - in: "-80x", - out: false, - }, { - in: "select", - out: false, - }} + // special cases + {"0", true}, // equal to "-" - for _, tcase := range testcases { - assert.Equal(t, IsKeyRange(tcase.in), tcase.out, tcase.in) + // invalid cases + {"", false}, // empty is not allowed + {"11", false}, // no hyphen + {"-1", false}, // odd number of digits + {"-111", false}, // odd number of digits + {"1-2", false}, // odd number of digits + {"x-80", false}, // invalid character + {"-80x", false}, // invalid character + {"select", false}, // nonsense + {"+", false}, // nonsense + } + for _, tt := range tests { + assert.Equalf(t, tt.want, IsValidKeyRange(tt.arg), "IsValidKeyRange(%v)", tt.arg) } } diff --git a/go/vt/topo/shard.go b/go/vt/topo/shard.go index 7f03bf13364..3219c2c3412 100644 --- a/go/vt/topo/shard.go +++ b/go/vt/topo/shard.go @@ -304,7 +304,7 @@ func (ts *Server) CreateShard(ctx context.Context, keyspace, shard string) (err return err } for _, si := range sis { - if si.KeyRange == nil || key.KeyRangesIntersect(si.KeyRange, keyRange) { + if si.KeyRange == nil || key.KeyRangeIntersect(si.KeyRange, keyRange) { value.IsPrimaryServing = false break } diff --git a/go/vt/topo/topoproto/srvkeyspace.go b/go/vt/topo/topoproto/srvkeyspace.go index 24618233fb2..88cfe4b9986 100644 --- a/go/vt/topo/topoproto/srvkeyspace.go +++ b/go/vt/topo/topoproto/srvkeyspace.go @@ -17,9 +17,10 @@ limitations under the License. package topoproto import ( - "bytes" "sort" + "vitess.io/vitess/go/vt/key" + topodatapb "vitess.io/vitess/go/vt/proto/topodata" ) @@ -29,18 +30,12 @@ type ShardReferenceArray []*topodatapb.ShardReference // Len implements sort.Interface func (sra ShardReferenceArray) Len() int { return len(sra) } -// Len implements sort.Interface +// Less implements sort.Interface func (sra ShardReferenceArray) Less(i, j int) bool { - if sra[i].KeyRange == nil || len(sra[i].KeyRange.Start) == 0 { - return true - } - if sra[j].KeyRange == nil || len(sra[j].KeyRange.Start) == 0 { - return false - } - return bytes.Compare(sra[i].KeyRange.Start, sra[j].KeyRange.Start) < 0 + return key.KeyRangeLess(sra[i].KeyRange, sra[j].KeyRange) } -// Len implements sort.Interface +// Swap implements sort.Interface func (sra ShardReferenceArray) Swap(i, j int) { sra[i], sra[j] = sra[j], sra[i] } diff --git a/go/vt/topotools/split.go b/go/vt/topotools/split.go index 00cfd49cdae..ace3dda94a7 100644 --- a/go/vt/topotools/split.go +++ b/go/vt/topotools/split.go @@ -222,7 +222,7 @@ func findOverlappingShards(shardMap map[string]*topo.ShardInfo) ([]*OverlappingS func findIntersectingShard(shardMap map[string]*topo.ShardInfo, sourceArray []*topo.ShardInfo) *topo.ShardInfo { for name, si := range shardMap { for _, sourceShardInfo := range sourceArray { - if si.KeyRange == nil || sourceShardInfo.KeyRange == nil || key.KeyRangesIntersect(si.KeyRange, sourceShardInfo.KeyRange) { + if si.KeyRange == nil || sourceShardInfo.KeyRange == nil || key.KeyRangeIntersect(si.KeyRange, sourceShardInfo.KeyRange) { delete(shardMap, name) return si } @@ -235,7 +235,7 @@ func findIntersectingShard(shardMap map[string]*topo.ShardInfo, sourceArray []*t // in the destination array func intersect(si *topo.ShardInfo, allShards []*topo.ShardInfo) bool { for _, shard := range allShards { - if key.KeyRangesIntersect(si.KeyRange, shard.KeyRange) { + if key.KeyRangeIntersect(si.KeyRange, shard.KeyRange) { return true } } diff --git a/go/vt/topotools/split_test.go b/go/vt/topotools/split_test.go index 0ba13a3524d..003dc767317 100644 --- a/go/vt/topotools/split_test.go +++ b/go/vt/topotools/split_test.go @@ -129,6 +129,10 @@ func TestValidateForReshard(t *testing.T) { sources: []string{"0"}, targets: []string{"-40", "40-"}, out: "", + }, { + sources: []string{"0003-"}, + targets: []string{"000300-000380", "000380-000400", "0004-"}, + out: "", }, { sources: []string{"-40", "40-80", "80-"}, targets: []string{"-40", "40-"}, diff --git a/go/vt/vtctl/workflow/stream_migrator.go b/go/vt/vtctl/workflow/stream_migrator.go index c5e45a06bb2..096c800a53d 100644 --- a/go/vt/vtctl/workflow/stream_migrator.go +++ b/go/vt/vtctl/workflow/stream_migrator.go @@ -653,7 +653,7 @@ func (sm *StreamMigrator) templatizeRule(ctx context.Context, rule *binlogdatapb switch { case rule.Filter == "": return StreamTypeUnknown, fmt.Errorf("rule %v does not have a select expression in vreplication", rule) - case key.IsKeyRange(rule.Filter): + case key.IsValidKeyRange(rule.Filter): rule.Filter = "{{.}}" return StreamTypeSharded, nil case rule.Filter == vreplication.ExcludeStr: diff --git a/go/vt/vttablet/tabletmanager/vdiff/workflow_differ.go b/go/vt/vttablet/tabletmanager/vdiff/workflow_differ.go index cfc8e620a93..c666c017cef 100644 --- a/go/vt/vttablet/tabletmanager/vdiff/workflow_differ.go +++ b/go/vt/vttablet/tabletmanager/vdiff/workflow_differ.go @@ -236,7 +236,7 @@ func (wd *workflowDiffer) buildPlan(dbClient binlogplayer.DBClient, filter *binl buf := sqlparser.NewTrackedBuffer(nil) buf.Myprintf("select * from %v", sqlparser.NewIdentifierCS(table.Name)) sourceQuery = buf.String() - case key.IsKeyRange(rule.Filter): + case key.IsValidKeyRange(rule.Filter): buf := sqlparser.NewTrackedBuffer(nil) buf.Myprintf("select * from %v where in_keyrange(%v)", sqlparser.NewIdentifierCS(table.Name), sqlparser.NewStrLiteral(rule.Filter)) sourceQuery = buf.String() diff --git a/go/vt/vttablet/tabletmanager/vreplication/table_plan_builder.go b/go/vt/vttablet/tabletmanager/vreplication/table_plan_builder.go index d4a74540d3d..f2aeafb6e7b 100644 --- a/go/vt/vttablet/tabletmanager/vreplication/table_plan_builder.go +++ b/go/vt/vttablet/tabletmanager/vreplication/table_plan_builder.go @@ -201,7 +201,7 @@ func buildTablePlan(tableName string, rule *binlogdatapb.Rule, colInfos []*Colum buf := sqlparser.NewTrackedBuffer(nil) buf.Myprintf("select * from %v", sqlparser.NewIdentifierCS(tableName)) query = buf.String() - case key.IsKeyRange(filter): + case key.IsValidKeyRange(filter): buf := sqlparser.NewTrackedBuffer(nil) buf.Myprintf("select * from %v where in_keyrange(%v)", sqlparser.NewIdentifierCS(tableName), sqlparser.NewStrLiteral(filter)) query = buf.String() diff --git a/go/vt/vttablet/tabletserver/vstreamer/uvstreamer.go b/go/vt/vttablet/tabletserver/vstreamer/uvstreamer.go index 40bf27dd0cf..676388b1517 100644 --- a/go/vt/vttablet/tabletserver/vstreamer/uvstreamer.go +++ b/go/vt/vttablet/tabletserver/vstreamer/uvstreamer.go @@ -216,7 +216,7 @@ func getQuery(tableName string, filter string) string { buf := sqlparser.NewTrackedBuffer(nil) buf.Myprintf("select * from %v", sqlparser.NewIdentifierCS(tableName)) query = buf.String() - case key.IsKeyRange(filter): + case key.IsValidKeyRange(filter): buf := sqlparser.NewTrackedBuffer(nil) buf.Myprintf("select * from %v where in_keyrange(%v)", sqlparser.NewIdentifierCS(tableName), sqlparser.NewStrLiteral(filter)) query = buf.String() diff --git a/go/vt/wrangler/materializer.go b/go/vt/wrangler/materializer.go index fdd4b2c3007..231ec3f5bae 100644 --- a/go/vt/wrangler/materializer.go +++ b/go/vt/wrangler/materializer.go @@ -1224,7 +1224,7 @@ func (mz *materializer) generateInserts(ctx context.Context, targetShard *topo.S // We only do it for MoveTables for now since this doesn't hold for materialize flows // where the target's sharding key might differ from that of the source if mz.ms.MaterializationIntent == vtctldatapb.MaterializationIntent_MOVETABLES && - !key.KeyRangesIntersect(sourceShard.KeyRange, targetShard.KeyRange) { + !key.KeyRangeIntersect(sourceShard.KeyRange, targetShard.KeyRange) { continue } bls := &binlogdatapb.BinlogSource{ diff --git a/go/vt/wrangler/resharder.go b/go/vt/wrangler/resharder.go index 859651b270b..097a174de18 100644 --- a/go/vt/wrangler/resharder.go +++ b/go/vt/wrangler/resharder.go @@ -313,7 +313,7 @@ func (rs *resharder) createStreams(ctx context.Context) error { // copy excludeRules to prevent data race. copyExcludeRules := append([]*binlogdatapb.Rule(nil), excludeRules...) for _, source := range rs.sourceShards { - if !key.KeyRangesIntersect(target.KeyRange, source.KeyRange) { + if !key.KeyRangeIntersect(target.KeyRange, source.KeyRange) { continue } filter := &binlogdatapb.Filter{ diff --git a/go/vt/wrangler/traffic_switcher.go b/go/vt/wrangler/traffic_switcher.go index 79a967121ae..b28fbeb118f 100644 --- a/go/vt/wrangler/traffic_switcher.go +++ b/go/vt/wrangler/traffic_switcher.go @@ -949,9 +949,7 @@ func (ts *trafficSwitcher) isPartialMoveTables(sourceShards, targetShards []stri return false, err } - if !key.KeyRangeIsPartial(skr) || !key.KeyRangeIsPartial(tkr) || // both cover full range - len(sourceShards) != len(targetShards) { - + if key.KeyRangeIsComplete(skr) || key.KeyRangeIsComplete(tkr) || len(sourceShards) != len(targetShards) { return false, nil } diff --git a/go/vt/wrangler/traffic_switcher_env_test.go b/go/vt/wrangler/traffic_switcher_env_test.go index af0a6bea2f3..201d76c0955 100644 --- a/go/vt/wrangler/traffic_switcher_env_test.go +++ b/go/vt/wrangler/traffic_switcher_env_test.go @@ -343,7 +343,7 @@ func newTestShardMigrater(ctx context.Context, t *testing.T, sourceShards, targe var rows, rowsRdOnly []string var streamExtInfoRows []string for j, sourceShard := range sourceShards { - if !key.KeyRangesIntersect(tme.targetKeyRanges[i], tme.sourceKeyRanges[j]) { + if !key.KeyRangeIntersect(tme.targetKeyRanges[i], tme.sourceKeyRanges[j]) { continue } bls := &binlogdatapb.BinlogSource{ @@ -490,7 +490,7 @@ func (tme *testMigraterEnv) expectNoPreviousReverseJournals() { func (tme *testShardMigraterEnv) forAllStreams(f func(i, j int)) { for i := range tme.targetShards { for j := range tme.sourceShards { - if !key.KeyRangesIntersect(tme.targetKeyRanges[i], tme.sourceKeyRanges[j]) { + if !key.KeyRangeIntersect(tme.targetKeyRanges[i], tme.sourceKeyRanges[j]) { continue } f(i, j) diff --git a/go/vt/wrangler/vdiff.go b/go/vt/wrangler/vdiff.go index cfecefaeabd..e77ca253a84 100644 --- a/go/vt/wrangler/vdiff.go +++ b/go/vt/wrangler/vdiff.go @@ -398,7 +398,7 @@ func (df *vdiff) buildVDiffPlan(ctx context.Context, filter *binlogdatapb.Filter continue } query := rule.Filter - if rule.Filter == "" || key.IsKeyRange(rule.Filter) { + if rule.Filter == "" || key.IsValidKeyRange(rule.Filter) { buf := sqlparser.NewTrackedBuffer(nil) buf.Myprintf("select * from %v", sqlparser.NewIdentifierCS(table.Name)) query = buf.String()