diff --git a/fragment.go b/fragment.go index 654ba5b18..aa93dcc9a 100644 --- a/fragment.go +++ b/fragment.go @@ -2388,7 +2388,45 @@ func (p *parallelSlices) fullPrune() { } unsorted := p.prune() if unsorted { + + // Q: why sort.Stable instead of sort.Sort? + // + // A: Because we need to ensure that the last entry + // is the one that wins. The last entry is the most recent update, and + // so the mutex field should reflect that one and not earlier + // updates. Mutex fields are special in that only 1 bit can + // be hot (set to 1), so the last update overrides all the others. We + // exploit this to eliminate irrelevant earlier writes. + // + // So if the input columns were {1, 2, 1}, + // and input rows were {3, 4, 5}, + // we need to be sure that we end up with cols:{1, 2} + // rows:{5, 4} // right, last update won. + // and not rows:{3, 4} // wrong, first update won. + // + // illustrated: (dk = don't know state) + // + // the new, raw data before later updates "win": + // + // col0 col1 col2 + // row3 dk 1 dk + // row4 dk dk 1 + // row5 dk 1 dk + // + // after last one wins, to be written to the backend: + // + // col0 col1 col2 + // row3 dk 0 0 + // row4 dk 0 1 + // row5 dk 1 0 + // ^ + // \-- on col1. The last one won, b/c its a mutex all other rows go to 0 for that column. + // + // Which means we need a stable sort, ensuring that if two things + // have the same column key, they stay in the same relative order, + // and then the prune algorithm always keeps the last. sort.Stable(p) + _ = p.prune() } } diff --git a/roaring/filter_internal_test.go b/roaring/filter_internal_test.go index 54ee01b7c..0fc9c0fe0 100644 --- a/roaring/filter_internal_test.go +++ b/roaring/filter_internal_test.go @@ -16,6 +16,8 @@ package roaring import ( "fmt" + "math/rand" + "reflect" "sort" "sync" "testing" @@ -147,6 +149,129 @@ func TestBitmapFilter(t *testing.T) { } } +func TestNewBitmapBitmapFilter_static(t *testing.T) { + + bm := NewBitmap(1<<16, 2<<16, 5<<16) + + positions := make([]uint64, 0, 80) + callback := func(pos uint64) error { + positions = append(positions, pos) + return nil + } + bbfilt := NewBitmapBitmapFilter(bm, callback) + + // now verify nextOffsets with a slightly different algorithm + + containers := make([]*Container, rowWidth) + algoExpectedNextOffsets := make([]uint64, rowWidth) + + iter, _ := bm.Containers.Iterator(0) + last := uint64(0) + first := uint64(0) + count := 0 + for iter.Next() { + k, v := iter.Value() + // Coerce container key into the 0-rowWidth range we'll be + // using to compare against containers within each row. + k = k & keyMask + + // we only have one row in filterColumns, so we won't be overwriting anything. + containers[k] = v + + last = k + if count == 0 { + first = k + } + count++ + } + + // last = 5; first = 1 + // bbfilt.containers: [ - 1 2 - - 5 - -... (all - to end) ] + // nextOffsets: [ 1 2 5 5 5 1 1 1...(all 1s to end) ] desired + curLast := last + for i := rowWidth - 1; i >= 0; i-- { + if uint64(i) >= last { + algoExpectedNextOffsets[i] = first + } else { + algoExpectedNextOffsets[i] = curLast + if containers[i] != nil { + curLast = uint64(i) + } + } + } + + // compare observed and algoExpectedNextOffsets: + if !reflect.DeepEqual(bbfilt.nextOffsets, algoExpectedNextOffsets) { + t.Errorf("observed bbfilt.nextOffsets: %v, expected %v", bbfilt.nextOffsets, algoExpectedNextOffsets) + } +} + +func TestNewBitmapBitmapFilter_random(t *testing.T) { + + rand.Seed(1) + for N := 0; N < 100; N++ { + bm := NewBitmap() + shardWidth := uint64(rowWidth << 16) + _ = shardWidth + for i := 0; i < N; i++ { + _, _ = bm.AddN(rand.Uint64() % shardWidth) + } + + positions := make([]uint64, 0, 80) + callback := func(pos uint64) error { + positions = append(positions, pos) + return nil + } + bbfilt := NewBitmapBitmapFilter(bm, callback) + + // now verify nextOffsets with a slightly different algorithm + + containers := make([]*Container, rowWidth) + algoExpectedNextOffsets := make([]uint64, rowWidth) + + iter, _ := bm.Containers.Iterator(0) + last := uint64(0) + first := uint64(0) + count := 0 + for iter.Next() { + k, v := iter.Value() + // Coerce container key into the 0-rowWidth range we'll be + // using to compare against containers within each row. + k = k & keyMask + + // we only have one row in filterColumns, so we won't be overwriting anything. + containers[k] = v + + last = k + if count == 0 { + first = k + } + count++ + } + + // small example + // last = 5; first = 1 + // bbfilt.containers: [ - 1 2 - - 5 - -... (all - to end) ] + // nextOffsets: [ 1 2 5 5 5 1 1 1...(all 1s to end) ] desired + curLast := last + for i := rowWidth - 1; i >= 0; i-- { + if uint64(i) >= last { + algoExpectedNextOffsets[i] = first + } else { + algoExpectedNextOffsets[i] = curLast + if containers[i] != nil { + curLast = uint64(i) + } + } + } + + // compare observed and algoExpectedNextOffsets: + if !reflect.DeepEqual(bbfilt.nextOffsets, algoExpectedNextOffsets) { + t.Errorf("observed bbfilt.nextOffsets: %v, expected %v", bbfilt.nextOffsets, algoExpectedNextOffsets) + } + } +} + func TestLimitFilter(t *testing.T) { f := NewBitmapRowLimitFilter(5)