Skip to content

Commit

Permalink
Fix off by one issue with limit option
Browse files Browse the repository at this point in the history
  • Loading branch information
ostafen committed Apr 15, 2024
1 parent 8a17144 commit a68b05b
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 54 deletions.
21 changes: 2 additions & 19 deletions pkg/database/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -1473,7 +1473,6 @@ func (d *db) TxScan(ctx context.Context, req *schema.TxScanRequest) (*schema.TxL
defer d.releaseTx(tx)

limit := int(req.Limit)

if req.Limit == 0 {
limit = d.maxResultSize
}
Expand Down Expand Up @@ -1506,13 +1505,6 @@ func (d *db) TxScan(ctx context.Context, req *schema.TxScanRequest) (*schema.TxL
}

txList.Txs = append(txList.Txs, sTx)

if l == d.maxResultSize {
return txList,
fmt.Errorf("%w: found at least %d entries (maximum limit). "+
"Pagination over large results can be achieved by using the limit and initialTx arguments",
ErrResultSizeLimitReached, d.maxResultSize)
}
}

return txList, nil
Expand Down Expand Up @@ -1546,14 +1538,13 @@ func (d *db) History(ctx context.Context, req *schema.HistoryRequest) (*schema.E
}

limit := int(req.Limit)

if req.Limit == 0 {
if limit == 0 {
limit = d.maxResultSize
}

key := EncodeKey(req.Key)

valRefs, hCount, err := d.st.History(key, req.Offset, req.Desc, limit)
valRefs, _, err := d.st.History(key, req.Offset, req.Desc, limit)
if err != nil && err != store.ErrOffsetOutOfRange {
return nil, err
}
Expand All @@ -1580,14 +1571,6 @@ func (d *db) History(ctx context.Context, req *schema.HistoryRequest) (*schema.E
Revision: valRef.HC(),
}
}

if limit == d.maxResultSize && hCount >= uint64(d.maxResultSize) {
return list,
fmt.Errorf("%w: found at least %d entries (the maximum limit). "+
"Pagination over large results can be achieved by using the limit and initialTx arguments",
ErrResultSizeLimitReached, d.maxResultSize)
}

return list, nil
}

Expand Down
8 changes: 4 additions & 4 deletions pkg/database/database_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -914,7 +914,7 @@ func TestTxScan(t *testing.T) {
},
},
})
require.ErrorIs(t, err, ErrResultSizeLimitReached)
require.NoError(t, err)
require.Len(t, txList.Txs, len(kvs))

for i := 0; i < len(kvs); i++ {
Expand Down Expand Up @@ -952,7 +952,7 @@ func TestTxScan(t *testing.T) {
txList, err := db.TxScan(context.Background(), &schema.TxScanRequest{
InitialTx: initialState.TxId + 1,
})
require.ErrorIs(t, err, ErrResultSizeLimitReached)
require.NoError(t, err)
require.Len(t, txList.Txs, len(kvs))

for i := 0; i < len(kvs); i++ {
Expand Down Expand Up @@ -1008,7 +1008,7 @@ func TestHistory(t *testing.T) {
Key: kvs[0].Key,
SinceTx: lastTx,
})
require.ErrorIs(t, err, ErrResultSizeLimitReached)
require.NoError(t, err)

for i, val := range inc.Entries {
require.Equal(t, kvs[0].Key, val.Key)
Expand All @@ -1025,7 +1025,7 @@ func TestHistory(t *testing.T) {
SinceTx: lastTx,
Desc: true,
})
require.ErrorIs(t, err, ErrResultSizeLimitReached)
require.NoError(t, err)

for i, val := range dec.Entries {
require.Equal(t, kvs[0].Key, val.Key)
Expand Down
8 changes: 0 additions & 8 deletions pkg/database/scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ func (d *db) Scan(ctx context.Context, req *schema.ScanRequest) (*schema.Entries
}

limit := int(req.Limit)

if req.Limit == 0 {
limit = d.maxResultSize
}
Expand Down Expand Up @@ -100,13 +99,6 @@ func (d *db) Scan(ctx context.Context, req *schema.ScanRequest) (*schema.Entries
}

entries.Entries = append(entries.Entries, e)

if l == d.maxResultSize {
return entries,
fmt.Errorf("%w: found at least %d entries (the maximum limit). "+
"Pagination over large results can be achieved by using the limit and initialTx arguments",
ErrResultSizeLimitReached, d.maxResultSize)
}
}

return entries, nil
Expand Down
28 changes: 18 additions & 10 deletions pkg/database/scan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,11 @@ func TestStoreScan(t *testing.T) {

db.maxResultSize = 3

db.Set(context.Background(), &schema.SetRequest{KVs: []*schema.KeyValue{{Key: []byte(`aaa`), Value: []byte(`item1`)}}})
db.Set(context.Background(), &schema.SetRequest{KVs: []*schema.KeyValue{{Key: []byte(`bbb`), Value: []byte(`item2`)}}})
_, err := db.Set(context.Background(), &schema.SetRequest{KVs: []*schema.KeyValue{{Key: []byte(`aaa`), Value: []byte(`item1`)}}})
require.NoError(t, err)

_, err = db.Set(context.Background(), &schema.SetRequest{KVs: []*schema.KeyValue{{Key: []byte(`bbb`), Value: []byte(`item2`)}}})
require.NoError(t, err)

scanOptions := schema.ScanRequest{
Prefix: []byte(`z`),
Expand All @@ -51,6 +54,9 @@ func TestStoreScan(t *testing.T) {
_, err = db.Scan(context.Background(), nil)
require.Equal(t, store.ErrIllegalArguments, err)

_, err = db.Set(context.Background(), &schema.SetRequest{KVs: []*schema.KeyValue{{Key: []byte(`acb`), Value: []byte(`item4`)}}})
require.NoError(t, err)

scanOptions = schema.ScanRequest{
SeekKey: []byte(`b`),
Prefix: []byte(`a`),
Expand All @@ -70,11 +76,13 @@ func TestStoreScan(t *testing.T) {

list, err = db.Scan(context.Background(), &scanOptions)
require.NoError(t, err)
require.Exactly(t, 2, len(list.Entries))
require.Equal(t, list.Entries[0].Key, []byte(`abc`))
require.Equal(t, list.Entries[0].Value, []byte(`item3`))
require.Equal(t, list.Entries[1].Key, []byte(`aaa`))
require.Equal(t, list.Entries[1].Value, []byte(`item1`))
require.Exactly(t, 3, len(list.Entries))
require.Equal(t, list.Entries[0].Key, []byte(`acb`))
require.Equal(t, list.Entries[0].Value, []byte(`item4`))
require.Equal(t, list.Entries[1].Key, []byte(`abc`))
require.Equal(t, list.Entries[1].Value, []byte(`item3`))
require.Equal(t, list.Entries[2].Key, []byte(`aaa`))
require.Equal(t, list.Entries[2].Value, []byte(`item1`))

scanOptions1 := schema.ScanRequest{
SeekKey: []byte(`a`),
Expand All @@ -84,14 +92,14 @@ func TestStoreScan(t *testing.T) {
}

list1, err := db.Scan(context.Background(), &scanOptions1)
require.ErrorIs(t, err, ErrResultSizeLimitReached)
require.NoError(t, err)
require.Exactly(t, 3, len(list1.Entries))
require.Equal(t, list1.Entries[0].Key, []byte(`aaa`))
require.Equal(t, list1.Entries[0].Value, []byte(`item1`))
require.Equal(t, list1.Entries[1].Key, []byte(`abc`))
require.Equal(t, list1.Entries[1].Value, []byte(`item3`))
require.Equal(t, list1.Entries[2].Key, []byte(`bbb`))
require.Equal(t, list1.Entries[2].Value, []byte(`item2`))
require.Equal(t, list1.Entries[2].Key, []byte(`acb`))
require.Equal(t, list1.Entries[2].Value, []byte(`item4`))
}

func TestStoreScanPrefix(t *testing.T) {
Expand Down
8 changes: 0 additions & 8 deletions pkg/database/sorted_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ func (d *db) ZScan(ctx context.Context, req *schema.ZScanRequest) (*schema.ZEntr
}

limit := int(req.Limit)

if req.Limit == 0 {
limit = d.maxResultSize
}
Expand Down Expand Up @@ -185,7 +184,6 @@ func (d *db) ZScan(ctx context.Context, req *schema.ZScanRequest) (*schema.ZEntr
defer kvsnap.Close()

entries := &schema.ZEntries{}

for l := 1; l <= limit; l++ {
zKey, _, err := r.Read(ctx)
if errors.Is(err, store.ErrNoMoreEntries) {
Expand All @@ -195,12 +193,6 @@ func (d *db) ZScan(ctx context.Context, req *schema.ZScanRequest) (*schema.ZEntr
return nil, err
}

if l == d.maxResultSize {
return entries, fmt.Errorf("%w: found at least %d entries (the maximum limit). "+
"Pagination over large results can be achieved by using the limit, seekKey, seekScore and seekAtTx arguments",
ErrResultSizeLimitReached, d.maxResultSize)
}

// zKey = [1+setLenLen+len(req.Set)+scoreLen+keyLenLen+1+len(req.Key)+txIDLen]
scoreOff := 1 + setLenLen + len(req.Set)
scoreB := binary.BigEndian.Uint64(zKey[scoreOff:])
Expand Down
2 changes: 1 addition & 1 deletion pkg/database/sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ func (d *db) SQLQueryPrepared(ctx context.Context, tx *sql.SQLTx, stmt sql.DataS
}

if l > d.maxResultSize {
return res, fmt.Errorf("%w: found at least %d rows (the maximum limit). "+
return res, fmt.Errorf("%w: found more than %d rows (the maximum limit). "+
"Query constraints can be applied using the LIMIT clause",
ErrResultSizeLimitReached, d.maxResultSize)
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/database/sql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func TestSQLExecAndQuery(t *testing.T) {
require.Len(t, res.Rows, 4)

ntx, ctxs, err = db.SQLExec(context.Background(), nil, &schema.SQLExecRequest{Sql: `
INSERT INTO table1(title, active, payload) VALUES ('title1', null, null), ('title2', true, null), ('title3', false, x'AADD')
INSERT INTO table1(title, active, payload) VALUES ('title1', null, null), ('title2', true, null), ('title3', false, x'AADD'), ('title4', false, x'ABCD')
`})
require.NoError(t, err)
require.Nil(t, ntx)
Expand All @@ -91,7 +91,7 @@ func TestSQLExecAndQuery(t *testing.T) {
require.NoError(t, err)
require.Len(t, res.Rows, 1)

q = "SELECT t.id, t.id as id2, title, active, payload FROM table1 t WHERE id <= 3 AND active != @active"
q = "SELECT t.id, t.id as id2, title, active, payload FROM table1 t WHERE id <= 4 AND active != @active"
res, err = db.SQLQuery(context.Background(), nil, &schema.SQLQueryRequest{Sql: q, Params: params})
require.ErrorIs(t, err, ErrResultSizeLimitReached)
require.Len(t, res.Rows, 2)
Expand Down Expand Up @@ -143,7 +143,7 @@ func TestSQLExecAndQuery(t *testing.T) {
_, err = db.VerifiableSQLGet(context.Background(), &schema.VerifiableSQLGetRequest{
SqlGetRequest: &schema.SQLGetRequest{
Table: "table1",
PkValues: []*schema.SQLValue{{Value: &schema.SQLValue_N{N: 4}}},
PkValues: []*schema.SQLValue{{Value: &schema.SQLValue_N{N: 5}}},
},
ProveSinceTx: 0,
})
Expand Down Expand Up @@ -173,7 +173,7 @@ func TestSQLExecAndQuery(t *testing.T) {
_, err = db.VerifiableSQLGet(context.Background(), &schema.VerifiableSQLGetRequest{
SqlGetRequest: &schema.SQLGetRequest{
Table: "table1",
PkValues: []*schema.SQLValue{{Value: &schema.SQLValue_N{N: 4}}},
PkValues: []*schema.SQLValue{{Value: &schema.SQLValue_N{N: 5}}},
},
ProveSinceTx: 0,
})
Expand Down

0 comments on commit a68b05b

Please sign in to comment.