From dbbdabef0550d877162905f7d38624d704f008db Mon Sep 17 00:00:00 2001 From: Matthew Jaffee Date: Mon, 21 Aug 2017 14:38:35 -0500 Subject: [PATCH 1/6] add failing test for XorRunRun --- roaring/roaring_internal_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/roaring/roaring_internal_test.go b/roaring/roaring_internal_test.go index 9cc2f2aa7..59db885e1 100644 --- a/roaring/roaring_internal_test.go +++ b/roaring/roaring_internal_test.go @@ -1832,6 +1832,11 @@ func TestXorRunRun(t *testing.T) { bruns: []interval16{{start: 2, last: 8}, {start: 16, last: 27}, {start: 33, last: 34}}, exp: []interval16{{start: 1, last: 1}, {start: 4, last: 4}, {start: 6, last: 6}, {start: 9, last: 9}, {start: 12, last: 15}, {start: 23, last: 27}, {start: 33, last: 34}}, }, + { + aruns: []interval16{{start: 65530, last: 65535}}, + bruns: []interval16{{start: 65531, last: 65535}}, + exp: []interval16{{start: 65530, last: 65530}}, + }, } for i, test := range tests { a.runs = test.aruns From 343ff04399ae50bd7b5d658397318de3d23d3433 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Tue, 22 Aug 2017 16:09:03 -0500 Subject: [PATCH 2/6] fixes #780;adjusted test to avoid array conversion --- roaring/roaring.go | 34 ++++++++++++++++++++++++-------- roaring/roaring_internal_test.go | 4 ++-- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/roaring/roaring.go b/roaring/roaring.go index 16e6159d7..266a9bfe3 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -3218,9 +3218,14 @@ func xorCompare(x *xorstm) (r1 interval16, has_data bool) { r1 = interval16{start: x.va.start, last: x.vb.start - 1} has_data = true } - x.va.start = x.vb.last + 1 - if x.va.start > x.va.last { + if x.vb.last == 65535 { x.va_valid = false + + } else { + x.va.start = x.vb.last + 1 + if x.va.start > x.va.last { + x.va_valid = false + } } } else if x.vb.start <= x.va.start && x.vb.last >= x.va.last { //va inside @@ -3230,26 +3235,39 @@ func xorCompare(x *xorstm) (r1 interval16, has_data bool) { has_data = true } - x.vb.start = x.va.last + 1 - if x.vb.start > x.vb.last { + if x.va.last == 65535 { x.vb_valid = false + } else { + x.vb.start = x.va.last + 1 + if x.vb.start > x.vb.last { + x.vb_valid = false + } } } else if x.va.start < x.vb.start && x.va.last <= x.vb.last { //va first overlap x.va_valid = false r1 = interval16{start: x.va.start, last: x.vb.start - 1} has_data = true - x.vb.start = x.va.last + 1 - if x.vb.start > x.vb.last { + if x.va.last == 65535 { x.vb_valid = false + } else { + x.vb.start = x.va.last + 1 + if x.vb.start > x.vb.last { + x.vb_valid = false + } } } else if x.vb.start < x.va.start && x.vb.last <= x.va.last { //vb first overlap x.vb_valid = false r1 = interval16{start: x.vb.start, last: x.va.start - 1} has_data = true - x.va.start = x.vb.last + 1 - if x.va.start > x.va.last { + + if x.vb.last == 65535 { x.va_valid = false + } else { + x.va.start = x.vb.last + 1 + if x.va.start > x.va.last { + x.va_valid = false + } } } return diff --git a/roaring/roaring_internal_test.go b/roaring/roaring_internal_test.go index 59db885e1..74dcc8992 100644 --- a/roaring/roaring_internal_test.go +++ b/roaring/roaring_internal_test.go @@ -1834,8 +1834,8 @@ func TestXorRunRun(t *testing.T) { }, { aruns: []interval16{{start: 65530, last: 65535}}, - bruns: []interval16{{start: 65531, last: 65535}}, - exp: []interval16{{start: 65530, last: 65530}}, + bruns: []interval16{{start: 65532, last: 65535}}, + exp: []interval16{{start: 65530, last: 65531}}, }, } for i, test := range tests { From 95d89e7648239e34ed60b66e52d922a3afc07940 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Wed, 23 Aug 2017 09:03:26 -0500 Subject: [PATCH 3/6] replaced magic number with constant; updated comments --- roaring/roaring.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/roaring/roaring.go b/roaring/roaring.go index 266a9bfe3..a42c09e4e 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -3218,7 +3218,8 @@ func xorCompare(x *xorstm) (r1 interval16, has_data bool) { r1 = interval16{start: x.va.start, last: x.vb.start - 1} has_data = true } - if x.vb.last == 65535 { + + if x.vb.last == maxContainerVal { // Check for overflow x.va_valid = false } else { @@ -3235,7 +3236,7 @@ func xorCompare(x *xorstm) (r1 interval16, has_data bool) { has_data = true } - if x.va.last == 65535 { + if x.va.last == maxContainerVal { //check for overflow x.vb_valid = false } else { x.vb.start = x.va.last + 1 @@ -3248,7 +3249,7 @@ func xorCompare(x *xorstm) (r1 interval16, has_data bool) { x.va_valid = false r1 = interval16{start: x.va.start, last: x.vb.start - 1} has_data = true - if x.va.last == 65535 { + if x.va.last == maxContainerVal { // check for overflow x.vb_valid = false } else { x.vb.start = x.va.last + 1 @@ -3261,7 +3262,7 @@ func xorCompare(x *xorstm) (r1 interval16, has_data bool) { r1 = interval16{start: x.vb.start, last: x.va.start - 1} has_data = true - if x.vb.last == 65535 { + if x.vb.last == maxContainerVal { // check for overflow x.va_valid = false } else { x.va.start = x.vb.last + 1 From ee3324ba8e2cbeef1956b91fd965c830151644dc Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Wed, 23 Aug 2017 11:20:33 -0500 Subject: [PATCH 4/6] overflow in arrayRun with supporting tests --- roaring/roaring.go | 15 +++++++++++---- roaring/roaring_internal_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/roaring/roaring.go b/roaring/roaring.go index a42c09e4e..903a04946 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -3152,8 +3152,11 @@ func xorArrayRun(a, b *container) *container { } else if va > vb.start { if va < vb.last { output.n += output.runAppendInterval(interval16{start: vb.start, last: va - 1}) - vb.start = va + 1 i++ + // candidate for overflow + // but no va must be less than max-1 + vb.start = va + 1 + if vb.start > vb.last { j++ } @@ -3162,15 +3165,19 @@ func xorArrayRun(a, b *container) *container { j++ } else { // va == vb.last vb.last-- - if vb.start < vb.last { + if vb.start <= vb.last { output.n += output.runAppendInterval(vb) } j++ i++ } - } else { - vb.start++ + } else { // we know va == vb.start + if vb.start == maxContainerVal { // protect overflow + j++ + } else { + vb.start++ + } i++ } } diff --git a/roaring/roaring_internal_test.go b/roaring/roaring_internal_test.go index 74dcc8992..6ee171fb4 100644 --- a/roaring/roaring_internal_test.go +++ b/roaring/roaring_internal_test.go @@ -1736,6 +1736,31 @@ func TestXorArrayRun(t *testing.T) { if !reflect.DeepEqual(ret.runs, expr) { t.Fatalf("test #4 expected %v, but got %v", exp, ret.array) } + + a = &container{array: []uint16{65535}, container_type: ContainerArray} + b = &container{runs: []interval16{{start: 65534, last: 65535}}, container_type: ContainerRun} + exp = []uint16{65534} + ret = xor(a, b) + if !reflect.DeepEqual(ret.array, exp) { + t.Fatalf("test #5 expected %v, but got %v", exp, ret.array) + } + + ret = xor(b, a) + if !reflect.DeepEqual(ret.array, exp) { + t.Fatalf("test #6 expected %v, but got %v", exp, ret.array) + } + + b = &container{runs: []interval16{{start: 65535, last: 65535}}, container_type: ContainerRun} + exp = []uint16{} + ret = xor(a, b) + if !reflect.DeepEqual(ret.array, exp) { + t.Fatalf("test #7 expected %v, but got %v", exp, ret.array) + } + + ret = xor(b, a) + if !reflect.DeepEqual(ret.array, exp) { + t.Fatalf("test #8 expected %v, but got %v", exp, ret.array) + } } //special case that didn't fit the xorrunrun table testing below. From 0f911498520eed43a1327fd52cf0ae1543e0a5f0 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Thu, 24 Aug 2017 13:47:58 -0500 Subject: [PATCH 5/6] converted xorArrayRun test to table; xor cardinality bug fix --- roaring/roaring.go | 3 ++ roaring/roaring_internal_test.go | 79 +++++++++++++------------------- 2 files changed, 36 insertions(+), 46 deletions(-) diff --git a/roaring/roaring.go b/roaring/roaring.go index 903a04946..4d862da64 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -3177,6 +3177,9 @@ func xorArrayRun(a, b *container) *container { j++ } else { vb.start++ + if vb.start > vb.last { + j++ + } } i++ } diff --git a/roaring/roaring_internal_test.go b/roaring/roaring_internal_test.go index 6ee171fb4..4c18bc5ef 100644 --- a/roaring/roaring_internal_test.go +++ b/roaring/roaring_internal_test.go @@ -1711,56 +1711,43 @@ func TestWriteReadRun(t *testing.T) { } func TestXorArrayRun(t *testing.T) { - a := &container{array: []uint16{1, 5, 10, 11, 12}, container_type: ContainerArray} - b := &container{runs: []interval16{{start: 2, last: 10}, {start: 12, last: 13}, {start: 15, last: 16}}, container_type: ContainerRun} - exp := []uint16{1, 2, 3, 4, 6, 7, 8, 9, 11, 13, 15, 16} - - //ret := xorArrayRun(a, b) - ret := xor(a, b) - if !reflect.DeepEqual(ret.array, exp) { - t.Fatalf("test #1 expected %v, but got %v", exp, ret.array) + tests := []struct { + a *container + b *container + exp *container + }{ + { + a: &container{array: []uint16{1, 5, 10, 11, 12}, container_type: ContainerArray}, + b: &container{runs: []interval16{{start: 2, last: 10}, {start: 12, last: 13}, {start: 15, last: 16}}, container_type: ContainerRun}, + exp: &container{array: []uint16{1, 2, 3, 4, 6, 7, 8, 9, 11, 13, 15, 16}, container_type: ContainerArray, n: 12}, + }, { + a: &container{array: []uint16{1, 5, 10, 11, 12, 13, 14}, container_type: ContainerArray}, + b: &container{runs: []interval16{{start: 2, last: 10}, {start: 12, last: 13}, {start: 15, last: 16}}, container_type: ContainerRun}, + exp: &container{array: []uint16{1, 2, 3, 4, 6, 7, 8, 9, 11, 14, 15, 16}, container_type: ContainerArray, n: 12}, + }, { + a: &container{array: []uint16{65535}, container_type: ContainerArray}, + b: &container{runs: []interval16{{start: 65534, last: 65535}}, container_type: ContainerRun}, + exp: &container{array: []uint16{65534}, container_type: ContainerArray, n: 1}, + }, { + a: &container{array: []uint16{65535}, container_type: ContainerArray}, + b: &container{runs: []interval16{{start: 65535, last: 65535}}, container_type: ContainerRun}, + exp: &container{array: []uint16{}, container_type: ContainerArray, n: 0}, + }, } - ret = xor(b, a) - if !reflect.DeepEqual(ret.array, exp) { - t.Fatalf("test #2 expected %v, but got %v", exp, ret.array) - } - c := &container{array: []uint16{1, 5, 10, 11, 12, 13, 14}, container_type: ContainerArray} - // exp = []int16{1, 2, 3, 4, 6, 7, 8, 9, 11, 14, 15, 16} - expr := []interval16{{start: 1, last: 4}, {start: 6, last: 9}, {start: 11, last: 11}, {start: 14, last: 16}} - ret = xor(b, c) - if !reflect.DeepEqual(ret.runs, expr) { - t.Fatalf("test #3 expected %v, but got %v", exp, ret.runs) - } - ret = xor(c, b) - if !reflect.DeepEqual(ret.runs, expr) { - t.Fatalf("test #4 expected %v, but got %v", exp, ret.array) + for i, test := range tests { + test.a.n = test.a.count() + test.b.n = test.b.count() + ret := xor(test.a, test.b) + if !reflect.DeepEqual(ret, test.exp) { + t.Fatalf("test #%v expected %v, but got %v", i, test.exp, ret) + } + ret = xor(test.b, test.a) + if !reflect.DeepEqual(ret, test.exp) { + t.Fatalf("test #%v.1 expected %v, but got %v", i, test.exp, ret) + } } - a = &container{array: []uint16{65535}, container_type: ContainerArray} - b = &container{runs: []interval16{{start: 65534, last: 65535}}, container_type: ContainerRun} - exp = []uint16{65534} - ret = xor(a, b) - if !reflect.DeepEqual(ret.array, exp) { - t.Fatalf("test #5 expected %v, but got %v", exp, ret.array) - } - - ret = xor(b, a) - if !reflect.DeepEqual(ret.array, exp) { - t.Fatalf("test #6 expected %v, but got %v", exp, ret.array) - } - - b = &container{runs: []interval16{{start: 65535, last: 65535}}, container_type: ContainerRun} - exp = []uint16{} - ret = xor(a, b) - if !reflect.DeepEqual(ret.array, exp) { - t.Fatalf("test #7 expected %v, but got %v", exp, ret.array) - } - - ret = xor(b, a) - if !reflect.DeepEqual(ret.array, exp) { - t.Fatalf("test #8 expected %v, but got %v", exp, ret.array) - } } //special case that didn't fit the xorrunrun table testing below. From 72041eb4f22cdd01ce1183ef8f85b89f1a7e6f88 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Thu, 24 Aug 2017 13:49:43 -0500 Subject: [PATCH 6/6] comment cleanup --- roaring/roaring.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/roaring/roaring.go b/roaring/roaring.go index 4d862da64..a1b99fcbb 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -3153,8 +3153,6 @@ func xorArrayRun(a, b *container) *container { if va < vb.last { output.n += output.runAppendInterval(interval16{start: vb.start, last: va - 1}) i++ - // candidate for overflow - // but no va must be less than max-1 vb.start = va + 1 if vb.start > vb.last {