test and add commentary on synthetic suffix replacement on keys witho…

…ut suffix

This patch adds a few test cases to ensure that when the block iterator adds a
synthetic suffix to a key without a suffix, the block iterator calls behave as
expected.

sstable/block.go

@@ -432,10 +432,23 @@ type blockIter struct {
 	// blockB: a@1,b@2,c@1 with syntheticSuffix=3
 	//
 	// To ensure this, Suffix replacement will not change the ordering of keys in
-	// the block because the iter assumes the following about the block:
+	// the block because the iter assumes that no two keys in the block share the
+	// same prefix. Furthermore, during SeekGE and SeekLT operations, the block
+	// iterator handles "off by one" errors (explained in more detail in those
+	// functions) when, for a given key, originalSuffix < searchSuffix <
+	// replacementSuffix, with integer comparison. To handle these cases, the
+	// iterator assumes:
+	//
+	//  pebble.Compare(keyPrefix{replacementSuffix},keyPrefix{originalSuffix}) < 0
+	//  for keys with a suffix.
+	//
+	//  NB: it is possible for a block iter to add a synthetic suffix on a key
+	//  without a suffix, which implies
+	//  pebble.Compare(keyPrefix{replacementSuffix},keyPrefix{noSuffix}) > 0 ,
+	//  however, the iterator would never need to handle an off by one error in
+	//  this case since originalSuffix (empty) > searchSuffix (non empty), with
+	//  integer comparison.
 	//
-	// (1) no two keys in the block share the same prefix.
-	// (2) pebble.Compare(keyPrefix{replacementSuffix},keyPrefix{originalSuffix}) < 0
 	//
 	// In addition, we also assume that any block with rangekeys will not contain
 	// a synthetic suffix.

sstable/block_test.go

@@ -391,7 +391,6 @@ func (c *checker) check(
 }
 
 func TestBlockSyntheticSuffix(t *testing.T) {
-	// TODO(msbutler): add test where replacement suffix has fewer bytes than previous suffix
 	expectedSuffix := "15"
 	synthSuffix := []byte("@" + expectedSuffix)
 
@@ -405,7 +404,7 @@ func TestBlockSyntheticSuffix(t *testing.T) {
 
 			suffixWriter, expectedSuffixWriter := &blockWriter{restartInterval: restarts}, &blockWriter{restartInterval: restarts}
 			keys := []string{
-				"apple@2", "apricot@2", "banana@13",
+				"apple@2", "apricot@2", "banana@13", "cantaloupe",
 				"grape@2", "orange@14", "peach@4",
 				"pear@1", "persimmon@4",
 			}
@@ -476,6 +475,25 @@ func TestBlockSyntheticSuffix(t *testing.T) {
 			// Ensure off by one handling works at end of iterator
 			c.check(expect.Last())(got.Last())
 			c.check(expect.SeekLT([]byte("apple@10"), base.SeekLTFlagsNone))(got.SeekLT([]byte("apple@10"), base.SeekLTFlagsNone))
+
+			// Try searching on the suffixless key
+			c.check(expect.First())(got.First())
+			c.check(expect.SeekGE([]byte("cantaloupe"), base.SeekGEFlagsNone))(got.SeekGE([]byte("cantaloupe"), base.SeekGEFlagsNone))
+
+			c.check(expect.First())(got.First())
+			c.check(expect.SeekGE([]byte("cantaloupe@16"), base.SeekGEFlagsNone))(got.SeekGE([]byte("cantaloupe@16"), base.SeekGEFlagsNone))
+
+			c.check(expect.First())(got.First())
+			c.check(expect.SeekGE([]byte("cantaloupe@14"), base.SeekGEFlagsNone))(got.SeekGE([]byte("cantaloupe@14"), base.SeekGEFlagsNone))
+
+			c.check(expect.Last())(got.Last())
+			c.check(expect.SeekLT([]byte("cantaloupe"), base.SeekLTFlagsNone))(got.SeekLT([]byte("cantaloupe"), base.SeekLTFlagsNone))
+
+			c.check(expect.Last())(got.Last())
+			c.check(expect.SeekLT([]byte("cantaloupe10"), base.SeekLTFlagsNone))(got.SeekLT([]byte("cantaloupe10"), base.SeekLTFlagsNone))
+
+			c.check(expect.Last())(got.Last())
+			c.check(expect.SeekLT([]byte("cantaloupe16"), base.SeekLTFlagsNone))(got.SeekLT([]byte("cantaloupe16"), base.SeekLTFlagsNone))
 		})
 	}
 }