fix: Reverse order of records in memdb (#1075)

hermanschaaf · web-flow · commit 8356590c03f8 · 2023-07-05T15:24:27.000Z
Shuffling missed a case (I think) because we were using the same seed every time. Iterating over the items in reverse should have the same effect and is still deterministic. Same fix as in #1074 but also adds a test for it.
diff --git a/internal/memdb/memdb.go b/internal/memdb/memdb.go
@@ -3,7 +3,6 @@ package memdb
 import (
 	"context"
 	"fmt"
-	"math/rand"
 	"sync"
 
 	"github.com/apache/arrow/go/v13/arrow"
@@ -101,15 +100,12 @@ func (c *client) Read(_ context.Context, table *schema.Table, res chan<- arrow.R
 	defer c.memoryDBLock.RUnlock()
 
 	tableName := table.Name
-	// we randomize the order of the records here because we don't set an expectation
+	// we iterate over records in reverse here because we don't set an expectation
 	// of ordering on plugins, and we want to make sure that the tests are not
-	// dependent on the order of the records either.
+	// dependent on the order of insertion either.
 	rows := c.memoryDB[tableName]
-	rand.Shuffle(len(rows), func(i, j int) {
-		rows[i], rows[j] = rows[j], rows[i]
-	})
-	for _, row := range rows {
-		res <- row
+	for i := len(rows) - 1; i >= 0; i-- {
+		res <- rows[i]
 	}
 	return nil
 }
diff --git a/plugin/testing_write_insert.go b/plugin/testing_write_insert.go
@@ -124,6 +124,7 @@ func (s *WriterTestSuite) testInsertAll(ctx context.Context) error {
 	if err != nil {
 		return fmt.Errorf("failed to sync: %w", err)
 	}
+	sortRecords(table, readRecords, "id")
 
 	totalItems = TotalRows(readRecords)
 	if totalItems != 2 {

Original file line number	Diff line number	Diff line change
`@@ -124,6 +124,7 @@ func (s *WriterTestSuite) testInsertAll(ctx context.Context) error {`
`124`	`124`	`if err != nil {`
`125`	`125`	`return fmt.Errorf("failed to sync: %w", err)`
`126`	`126`	`}`
	`127`	`+ sortRecords(table, readRecords, "id")`
`127`	`128`
`128`	`129`	`totalItems = TotalRows(readRecords)`
`129`	`130`	`if totalItems != 2 {`