Skip to content

Commit

Permalink
colblk: fix UintBuilder inconsistent size bug
Browse files Browse the repository at this point in the history
This commit fixes the UintBuilder bug described in the previous
commit. We make the non-fast-path consistent with the fast path so
that the encoding can't change unexpectedly.
  • Loading branch information
RaduBerinde committed Oct 15, 2024
1 parent 2c80e44 commit 482762d
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 7 deletions.
16 changes: 9 additions & 7 deletions sstable/colblk/testdata/uints
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,7 @@ write
4:1000040
----

# The encoding pessimistically assumes that we may have a 0 value.
size rows=4 offset=0
----
Size(4, 0) = 20
Expand All @@ -560,16 +561,17 @@ write
5:44333222111
----

# Make sure the encoding hasn't changed.
size rows=4 offset=0
----
Size(4, 0) = 13
Size(4, 0) = 20

finish rows=4 offset=0
----
uints
├── 00-01: x 81 # encoding: 1b,delta
├── 01-09: x 40420f0000000000 # 64-bit constant: 1000000
├── 09-10: x 00 # data[0] = 0 + 1000000 = 1000000
├── 10-11: x 0a # data[1] = 10 + 1000000 = 1000010
├── 11-12: x 14 # data[2] = 20 + 1000000 = 1000020
└── 12-13: x 1e # data[3] = 30 + 1000000 = 1000030
├── 00-01: x 04 # encoding: 4b
├── 01-04: x 000000 # padding (aligning to 32-bit boundary)
├── 04-08: x 40420f00 # data[0] = 1000000
├── 08-12: x 4a420f00 # data[1] = 1000010
├── 12-16: x 54420f00 # data[2] = 1000020
└── 16-20: x 5e420f00 # data[3] = 1000030
6 changes: 6 additions & 0 deletions sstable/colblk/uints.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,12 @@ func (b *UintBuilder) determineEncoding(rows int) (_ UintEncoding, minimum uint6

// We have to recalculate the minimum and maximum.
minimum, maximum := computeMinMax(b.array.elems.Slice(rows))
if b.useDefault {
// Mirror the pessimism of the fast path so that the result is consistent.
// Otherwise, adding a row can result in a different encoding even when not
// including that row.
minimum = 0
}
return DetermineUintEncoding(minimum, maximum), minimum
}

Expand Down

0 comments on commit 482762d

Please sign in to comment.