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) + } +}