From cde954e12fe4301168ebe1c16d79b1484279cc55 Mon Sep 17 00:00:00 2001
From: Matt Jaffee <matthew.jaffee@gmail.com>
Date: Thu, 28 Mar 2019 15:07:16 -0500
Subject: [PATCH] importValue only considers the last instance of a column id

included test demonstrates bug
---
 fragment.go               |  8 +++++++-
 fragment_internal_test.go | 43 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/fragment.go b/fragment.go
index 5b861a53d..a0fc22c99 100644
--- a/fragment.go
+++ b/fragment.go
@@ -1743,6 +1743,7 @@ func (f *fragment) importValue(columnIDs, values []uint64, bitDepth uint, clear
 	var toSet, toClear []uint64
 
 	smallWrite := false
+	var colSet map[uint64]struct{}
 	if len(columnIDs)*int(bitDepth+1)+f.opN < f.MaxOpN {
 		smallWrite = true
 
@@ -1753,6 +1754,7 @@ func (f *fragment) importValue(columnIDs, values []uint64, bitDepth uint, clear
 		// slightly more than half of that to try to avoid reallocation.
 		toSet = make([]uint64, 0, len(columnIDs)*int(bitDepth+1)*(5/8))
 		toClear = make([]uint64, 0, len(columnIDs)*int(bitDepth+1)*(5/8))
+		colSet = make(map[uint64]struct{})
 	}
 
 	if !smallWrite {
@@ -1762,9 +1764,13 @@ func (f *fragment) importValue(columnIDs, values []uint64, bitDepth uint, clear
 	// Process every value.
 	// If an error occurs then reopen the storage.
 	if err := func() (err error) {
-		for i := range columnIDs {
+		for i := len(columnIDs) - 1; i >= 0; i-- {
 			columnID, value := columnIDs[i], values[i]
 			if smallWrite {
+				if _, ok := colSet[columnID]; ok {
+					continue
+				}
+				colSet[columnID] = struct{}{}
 				toSet, toClear, err = f.positionsForValue(columnID, bitDepth, value, clear, toSet, toClear)
 				if err != nil {
 					return errors.Wrap(err, "getting positions for value")
diff --git a/fragment_internal_test.go b/fragment_internal_test.go
index d553ca0fa..78e9be4bd 100644
--- a/fragment_internal_test.go
+++ b/fragment_internal_test.go
@@ -3070,3 +3070,46 @@ func TestImportValueConcurrent(t *testing.T) {
 		t.Fatalf("concurrently importing values: %v", err)
 	}
 }
+
+func TestImportMultipleValues(t *testing.T) {
+	tests := []struct {
+		cols      []uint64
+		vals      []uint64
+		checkCols []uint64
+		checkVals []uint64
+		depth     uint
+	}{
+		{
+			cols:      []uint64{0, 0},
+			vals:      []uint64{97, 100},
+			depth:     7,
+			checkCols: []uint64{0},
+			checkVals: []uint64{100},
+		},
+	}
+
+	for i, test := range tests {
+		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
+			f := mustOpenFragment("i", "f", viewBSIGroupPrefix+"foo", 0, CacheTypeNone)
+			err := f.importValue(test.cols, test.vals, test.depth, false)
+			if err != nil {
+				t.Fatalf("importing values: %v", err)
+			}
+
+			for i := range test.checkCols {
+				cc, cv := test.checkCols[i], test.checkVals[i]
+				n, exists, err := f.value(cc, test.depth)
+				if err != nil {
+					t.Fatalf("getting value: %v", err)
+				}
+				if !exists {
+					t.Errorf("column %d should exist", cc)
+				}
+				if n != 100 {
+					t.Errorf("wrong value: %d is not %d", n, cv)
+				}
+			}
+
+		})
+	}
+}