Skip to content

Commit 8356590

Browse files
authored
fix: Reverse order of records in memdb (#1075)
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.
1 parent a40598e commit 8356590

File tree

2 files changed

+5
-8
lines changed

2 files changed

+5
-8
lines changed

internal/memdb/memdb.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package memdb
33
import (
44
"context"
55
"fmt"
6-
"math/rand"
76
"sync"
87

98
"github.com/apache/arrow/go/v13/arrow"
@@ -101,15 +100,12 @@ func (c *client) Read(_ context.Context, table *schema.Table, res chan<- arrow.R
101100
defer c.memoryDBLock.RUnlock()
102101

103102
tableName := table.Name
104-
// we randomize the order of the records here because we don't set an expectation
103+
// we iterate over records in reverse here because we don't set an expectation
105104
// of ordering on plugins, and we want to make sure that the tests are not
106-
// dependent on the order of the records either.
105+
// dependent on the order of insertion either.
107106
rows := c.memoryDB[tableName]
108-
rand.Shuffle(len(rows), func(i, j int) {
109-
rows[i], rows[j] = rows[j], rows[i]
110-
})
111-
for _, row := range rows {
112-
res <- row
107+
for i := len(rows) - 1; i >= 0; i-- {
108+
res <- rows[i]
113109
}
114110
return nil
115111
}

plugin/testing_write_insert.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ func (s *WriterTestSuite) testInsertAll(ctx context.Context) error {
124124
if err != nil {
125125
return fmt.Errorf("failed to sync: %w", err)
126126
}
127+
sortRecords(table, readRecords, "id")
127128

128129
totalItems = TotalRows(readRecords)
129130
if totalItems != 2 {

0 commit comments

Comments
 (0)