From 57358088ec31fb651348a9497d7d7f2db1eef173 Mon Sep 17 00:00:00 2001 From: Marty Schoch Date: Wed, 6 May 2015 11:00:04 -0400 Subject: [PATCH] fix row merging bug trying to be clever, we reused the memory allocated for the left operand when doing partial merges this had been tested to be safe, in general. however, the implementation was then written such that we always reused globally defined operands, this meant that we mutated the operands which were intended to always represent +1/-1 this then cascades quickly to making increment/decrement values much larger/smaller than they should be related to #197 --- index/upside_down/row_merge.go | 5 +-- index/upside_down/row_merge_test.go | 52 +++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 index/upside_down/row_merge_test.go diff --git a/index/upside_down/row_merge.go b/index/upside_down/row_merge.go index fb9d987a..bb0e5e39 100644 --- a/index/upside_down/row_merge.go +++ b/index/upside_down/row_merge.go @@ -61,8 +61,9 @@ func (m *upsideDownMerge) FullMerge(key, existingValue []byte, operands [][]byte func (m *upsideDownMerge) PartialMerge(key, leftOperand, rightOperand []byte) ([]byte, bool) { left := int64(binary.LittleEndian.Uint64(leftOperand)) right := int64(binary.LittleEndian.Uint64(rightOperand)) - binary.LittleEndian.PutUint64(leftOperand, uint64(left+right)) - return leftOperand, true + rv := make([]byte, 8) + binary.LittleEndian.PutUint64(rv, uint64(left+right)) + return rv, true } func (m *upsideDownMerge) Name() string { diff --git a/index/upside_down/row_merge_test.go b/index/upside_down/row_merge_test.go new file mode 100644 index 00000000..eee3f071 --- /dev/null +++ b/index/upside_down/row_merge_test.go @@ -0,0 +1,52 @@ +// Copyright (c) 2014 Couchbase, Inc. +// Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file +// except in compliance with the License. You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software distributed under the +// License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, +// either express or implied. See the License for the specific language governing permissions +// and limitations under the License. + +package upside_down + +import ( + "bytes" + "encoding/binary" + "testing" +) + +func TestPartialMerge(t *testing.T) { + + tests := []struct { + in [][]byte + out uint64 + }{ + { + in: [][]byte{dictionaryTermIncr, dictionaryTermIncr, dictionaryTermIncr, dictionaryTermIncr, dictionaryTermIncr}, + out: 5, + }, + } + + mo := &upsideDownMerge{} + for _, test := range tests { + curr := test.in[0] + for _, next := range test.in[1:] { + var ok bool + curr, ok = mo.PartialMerge([]byte("key"), curr, next) + if !ok { + t.Errorf("expected partial merge ok") + } + } + actual := decodeCount(curr) + if actual != test.out { + t.Errorf("expected %d, got %d", test.out, actual) + } + } + +} + +func decodeCount(in []byte) uint64 { + buf := bytes.NewBuffer(in) + count, _ := binary.ReadUvarint(buf) + return count +}