Skip to content

Commit e19ac24

Browse files
committed
cockroachkvs: use bytes.Compare when comparing key version
This patch uses bytes.Compare to compare key versions instead of ComparePointSuffixes; the latter calls on `normalizeEngineSuffixForCompare` - which could end up strip additional bytes from the `suffix` (comparing only a prefix of the suffix) if the last byte of the version happens to match one of the sentinel lengths defined. Fixes: #158463
1 parent 36cb7a2 commit e19ac24

File tree

2 files changed

+50
-1
lines changed

2 files changed

+50
-1
lines changed

cockroachkvs/cockroachkvs.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -789,7 +789,17 @@ func (ks *cockroachKeySeeker) IsLowerBound(k []byte, syntheticSuffix []byte) boo
789789
// is either (a) unversioned (and sorts before all other suffixes) or (b) is
790790
// a non-MVCC key with an untyped version.
791791
if firstRowWall == 0 && firstLogical == 0 {
792-
return ComparePointSuffixes(ks.untypedVersions.At(0), version) >= 0
792+
firstRowUntypedVer := ks.untypedVersions.At(0)
793+
// Empty suffixes sort before non-empty suffixes.
794+
switch {
795+
case len(version) == 0:
796+
// This includes the case where both versions are empty.
797+
return true
798+
case len(firstRowUntypedVer) == 0:
799+
return false
800+
default:
801+
return bytes.Compare(version, firstRowUntypedVer) >= 0
802+
}
793803
}
794804

795805
var wallTime uint64

cockroachkvs/testdata/key_schema_key_seeker

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,3 +241,42 @@ IsLowerBound(moo @ 3000000000,1, "") = true
241241
IsLowerBound(moo @ 3000000000,0, "") = true
242242
IsLowerBound(moo @ 0000000000,1, "") = true
243243
IsLowerBound(moo @ 02073a83c45688420eaf97824255790f1e12, "") = true
244+
245+
# Regression test for cockroach/#158463. This test uses a lock table key (a
246+
# non-MVCC key with an untyped version) as the first key in the block.
247+
# Previously, we would incorrectly use the key version as a full suffix and
248+
# call ComparePointSuffixes with it.
249+
250+
define-block
251+
moo @ 02073a83c45688420eaf97824255790f1e
252+
moo @ 3000000002,1
253+
moo @ 3000000001,1
254+
moo @ 0000000001,0
255+
----
256+
Parse(moo @ 02073a83c45688420eaf97824255790f1e) = hex:6d6f6f0002073a83c45688420eaf97824255790f1e12
257+
Parse(moo @ 3000000002,1) = hex:6d6f6f0000000000b2d05e02000000010d
258+
Parse(moo @ 3000000001,1) = hex:6d6f6f0000000000b2d05e01000000010d
259+
Parse(moo @ 0000000001,0) = hex:6d6f6f00000000000000000109
260+
261+
is-lower-bound
262+
moo @ 03073a83c45688420eaf97824255790f1e
263+
moo @ 9000000000,2
264+
moo @ 8000000000,2
265+
moo @ 8000000000,1
266+
moo @ 8000000000,0
267+
moo @ 7000000000,9
268+
moo @ 3000000000,2
269+
moo @ 3000000000,1
270+
moo @ 3000000000,0
271+
moo @ 0000000000,1
272+
----
273+
IsLowerBound(moo @ 03073a83c45688420eaf97824255790f1e, "") = true
274+
IsLowerBound(moo @ 9000000000,2, "") = false
275+
IsLowerBound(moo @ 8000000000,2, "") = false
276+
IsLowerBound(moo @ 8000000000,1, "") = false
277+
IsLowerBound(moo @ 8000000000,0, "") = false
278+
IsLowerBound(moo @ 7000000000,9, "") = false
279+
IsLowerBound(moo @ 3000000000,2, "") = false
280+
IsLowerBound(moo @ 3000000000,1, "") = false
281+
IsLowerBound(moo @ 3000000000,0, "") = false
282+
IsLowerBound(moo @ 0000000000,1, "") = false

0 commit comments

Comments
 (0)