From d7718483bf4b4c259f17ede94a38433d9178285b Mon Sep 17 00:00:00 2001
From: Matthew Jaffee <jaffee@pilosa.com>
Date: Sat, 21 May 2022 08:25:32 -0500
Subject: [PATCH] fix bug where multiple ints in same batch could ingest union
 of ints

if the same ID is added multiple times with different values, only the
last value should get set. Without this change, if the multiple
records weren't immediately next to each other, all the bits from all
the values would get set.
---
 client/batch.go      |  4 +---
 client/batch_test.go | 46 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/client/batch.go b/client/batch.go
index 6161bcc2c..a35d426e0 100644
--- a/client/batch.go
+++ b/client/batch.go
@@ -1459,8 +1459,7 @@ type valsByIDsSortable struct {
 
 func (v *valsByIDsSortable) Len() int { return len(v.ids) }
 
-// comparing on shard rather than ID was twice as fast in informal tests
-func (v *valsByIDsSortable) Less(i, j int) bool { return v.ids[i]/v.width < v.ids[j]/v.width }
+func (v *valsByIDsSortable) Less(i, j int) bool { return v.ids[i] < v.ids[j] }
 func (v *valsByIDsSortable) Swap(i, j int) {
 	v.ids[i], v.ids[j] = v.ids[j], v.ids[i]
 	v.vals[i], v.vals[j] = v.vals[j], v.vals[i]
@@ -1550,7 +1549,6 @@ type rowsByIDsSortable struct {
 
 func (v *rowsByIDsSortable) Len() int { return len(v.ids) }
 
-// comparing on shard rather than ID was twice as fast in informal tests
 func (v *rowsByIDsSortable) Less(i, j int) bool { return v.ids[i] < v.ids[j] }
 func (v *rowsByIDsSortable) Swap(i, j int) {
 	v.ids[i], v.ids[j] = v.ids[j], v.ids[i]
diff --git a/client/batch_test.go b/client/batch_test.go
index 0507a66d0..7d8fd9e33 100644
--- a/client/batch_test.go
+++ b/client/batch_test.go
@@ -41,6 +41,7 @@ func TestAgainstCluster(t *testing.T) {
 	t.Run("test-import-batch-multiple-ints", func(t *testing.T) { testImportBatchMultipleInts(t, c, client) })
 	t.Run("test-import-batch-sets-clears", func(t *testing.T) { testImportBatchSetsAndClears(t, c, client) })
 	t.Run("test-topn-cache-regression", func(t *testing.T) { testTopNCacheRegression(t, c, client) })
+	t.Run("test-multiple-int-same-batch", func(t *testing.T) { testMultipleIntSameBatch(t, c, client) })
 }
 
 func testStringSliceCombos(t *testing.T, c *test.Cluster, client *Client) {
@@ -1520,3 +1521,48 @@ func testTopNCacheRegression(t *testing.T, c *test.Cluster, client *Client) {
 		t.Fatalf("unexpected topn result: %v", res)
 	}
 }
+
+// testMultipleIntSameBatch checks that if the same ID is added multiple times with different values that only the last value is set and the bits aren't mixed together. It adds a different ID in between the two same ones which triggered a bug because we were sorting by shard rather than ID.
+func testMultipleIntSameBatch(t *testing.T, c *test.Cluster, client *Client) {
+	schema := NewSchema()
+	idx := schema.Index("test-multiple-int-same-batch")
+	field := idx.Field("age", OptFieldTypeInt(0, 10000))
+	err := client.SyncSchema(schema)
+	if err != nil {
+		t.Fatalf("syncing schema: %v", err)
+	}
+
+	b, err := NewBatch(client, 4, idx, []*Field{field}, OptUseShardTransactionalEndpoint(true))
+	if err != nil {
+		t.Fatalf("getting batch: %v", err)
+	}
+
+	if err := b.Add(Row{
+		ID:     uint64(1),
+		Values: []interface{}{int64(1)},
+	}); err != nil {
+		t.Fatalf("adding to batch: %v", err)
+	}
+	if err := b.Add(Row{
+		ID:     uint64(2),
+		Values: []interface{}{int64(0)},
+	}); err != nil {
+		t.Fatalf("adding to batch: %v", err)
+	}
+	if err := b.Add(Row{
+		ID:     uint64(1),
+		Values: []interface{}{int64(2)},
+	}); err != nil {
+		t.Fatalf("adding to batch: %v", err)
+	}
+
+	if err := b.Import(); err != nil {
+		t.Fatalf("importing: %v", err)
+	}
+
+	if resp, err := client.Query(field.Sum(nil)); err != nil {
+		t.Fatalf("querying sum: %v", err)
+	} else if res := resp.Result().Value(); res != 2 {
+		t.Errorf("unexpected sum: %+v", res)
+	}
+}