Skip to content

Pagination keyset2 #956

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
2da0386
Keyset pagination
alexandrosfilios May 30, 2025
ce55e74
Merge branch 'pagination-keyset2' of https://github.com/hyperledger-l…
Jun 4, 2025
e475ba7
passing missing context
Jun 4, 2025
fbbd5e6
bug fixes in keyset pagination
Jun 5, 2025
8de3c0c
Some info
alexandrosfilios Jun 5, 2025
17f61d7
bug fix: keyset pagination handling lastId
Jun 5, 2025
7f43183
keyset pagination tests
Jun 8, 2025
f25ac45
bug fixes
Jun 8, 2025
9b5c53c
bug fix: dont let pagination add fieldName to select if it is already…
Jun 8, 2025
36ae33f
improved implementation
Jun 8, 2025
2da4b91
support asterix
Jun 8, 2025
61408de
fix keyset pagination with int id
Jun 12, 2025
20aa429
Keyset pagination
alexandrosfilios May 30, 2025
74c1d9c
bug fixes in keyset pagination
Jun 5, 2025
bbb88ba
Some info
alexandrosfilios Jun 5, 2025
db748b3
bug fix: keyset pagination handling lastId
Jun 5, 2025
7a259ac
keyset pagination tests
Jun 8, 2025
b55bfb2
bug fixes
Jun 8, 2025
70d032a
bug fix: dont let pagination add fieldName to select if it is already…
Jun 8, 2025
526150e
improved implementation
Jun 8, 2025
479a450
support asterix
Jun 8, 2025
38c05b0
fix keyset pagination with int id
Jun 12, 2025
11e4de3
Merge branch 'pagination-keyset2' of https://github.com/hyperledger-l…
Jun 12, 2025
65c23d9
fix some broken tests
Jun 12, 2025
1356ea6
bug fixes
Jun 15, 2025
1fe61b8
fixup!
alexandrosfilios Jun 17, 2025
2ac819c
debugging ... still not working
Jun 18, 2025
9452a81
pagination test with database works
Jun 23, 2025
af0c0dc
use switch instead of ifs
Jun 30, 2025
c86cf34
don't keep firstIdOffset and lastIdOffset in keyset pagination
Jun 30, 2025
aa71b57
typos and making sure a field added to sql query is unique
Jun 30, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions platform/common/driver/pagination.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ SPDX-License-Identifier: Apache-2.0

package driver

import (
"github.com/hyperledger-labs/fabric-smart-client/platform/common/utils/collections/iterators"
)
import "github.com/hyperledger-labs/fabric-smart-client/platform/common/utils/collections/iterators"

type Pagination interface {
Prev() (Pagination, error)
Expand Down
2 changes: 1 addition & 1 deletion platform/view/services/db/driver/sql/common/vault.go
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ func (db *vaultReader) GetAllTxStatuses(ctx context.Context, p driver.Pagination
if err != nil {
return nil, err
}
return &driver.PageIterator[*driver.TxStatus]{Items: txStatusIterator, Pagination: p}, nil
return pagination.NewPage(txStatusIterator, p)
}

func (db *vaultReader) queryStatus(ctx context.Context, where cond.Condition, pagination driver.Pagination) (driver.TxStatusIterator, error) {
Expand Down
3 changes: 3 additions & 0 deletions platform/view/services/db/driver/sql/query/common/field.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ package common

type Field Serializable

// FieldName is the name of the DB column
type FieldName string

func (n FieldName) WriteString(b Builder) {
b.WriteString(string(n))
}
Expand Down
16 changes: 12 additions & 4 deletions platform/view/services/db/driver/sql/query/common/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import (
"github.com/hyperledger-labs/fabric-smart-client/platform/common/driver"
)

// FieldName is the name of the DB column
type FieldName string
type OrderBy Serializable
type Condition ConditionSerializable

// Tuple is a tuple of parameters
type Tuple = []Param
Expand All @@ -30,10 +30,18 @@ type CondInterpreter interface {
InTuple(fields []Serializable, vals []Tuple, sb Builder)
}

type ModifiableQuery interface {
AddField(Field)
AddWhere(Condition)
AddOrderBy(OrderBy)
AddLimit(int)
AddOffset(int)
}

// PagInterpreter is the pagination interpreter
type PagInterpreter interface {
// Interpret modifies the SQL query to add pagination support
Interpret(p driver.Pagination, sb Builder)
// PreProcess modifies the SQL query to add pagination support
PreProcess(driver.Pagination, ModifiableQuery)
}

// Builder is the string builder
Expand Down
2 changes: 1 addition & 1 deletion platform/view/services/db/driver/sql/query/cond/andor.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func Or(cs ...Condition) Condition {
func newAndOr(conditions []Condition, trivialCondition Condition, operator string) Condition {
nonTrivial := make([]Condition, 0, len(conditions))
for _, c := range conditions {
if c != trivialCondition {
if c != nil && c != trivialCondition {
nonTrivial = append(nonTrivial, c)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ import (

type Tuple = common.Tuple

type Condition = common.ConditionSerializable
type Condition = common.Condition
Original file line number Diff line number Diff line change
Expand Up @@ -11,29 +11,51 @@ import (

"github.com/hyperledger-labs/fabric-smart-client/platform/common/driver"
"github.com/hyperledger-labs/fabric-smart-client/platform/view/services/db/driver/sql/query/common"
"github.com/hyperledger-labs/fabric-smart-client/platform/view/services/db/driver/sql/query/cond"
_select "github.com/hyperledger-labs/fabric-smart-client/platform/view/services/db/driver/sql/query/select"
)

func NewDefaultInterpreter() *interpreter {
return &interpreter{}
}

type Interpreter = common.PagInterpreter

type interpreter struct{}

func (i *interpreter) Interpret(p driver.Pagination, sb common.Builder) {
func (i *interpreter) PreProcess(p driver.Pagination, query common.ModifiableQuery) {
switch pagination := p.(type) {
case *none:
return

case *offset:
sb.WriteString(" LIMIT ").
WriteParam(pagination.pageSize).
WriteString(" OFFSET ").
WriteParam(pagination.offset)
query.AddLimit(pagination.pageSize)
query.AddOffset(pagination.offset)

case *keyset[string, any]:
query.AddField(pagination.sqlIdName)
query.AddOrderBy(_select.Asc(pagination.sqlIdName))
query.AddLimit(pagination.pageSize)
if pagination.firstId != pagination.nilElement() {
query.AddWhere(cond.CmpVal(pagination.sqlIdName, ">", pagination.firstId))
} else {
query.AddOffset(pagination.offset)
}

case *keyset[int, any]:
query.AddField(pagination.sqlIdName)
query.AddOrderBy(_select.Asc(pagination.sqlIdName))
query.AddLimit(pagination.pageSize)
if pagination.firstId != pagination.nilElement() {
query.AddWhere(cond.CmpVal(pagination.sqlIdName, ">", pagination.firstId))
} else {
query.AddOffset(pagination.offset)
}

case *empty:
sb.WriteString(" LIMIT 0 OFFSET 0")
return
query.AddLimit(0)
query.AddOffset(0)

default:
fmt.Printf("Type = %T\n", pagination)
panic(fmt.Sprintf("invalid pagination option %+v", pagination))
}
}
125 changes: 125 additions & 0 deletions platform/view/services/db/driver/sql/query/pagination/keyset.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
/*
Copyright IBM Corp. All Rights Reserved.

SPDX-License-Identifier: Apache-2.0
*/

package pagination

import (
"fmt"
"reflect"
"strings"

"github.com/hyperledger-labs/fabric-smart-client/platform/common/driver"
"github.com/hyperledger-labs/fabric-smart-client/platform/view/services/db/driver/sql/query/common"
)

// PropertyName is the name of the field in the struct that is returned from the database
// V is the type of the field
type PropertyName[V comparable] string

// ExtractField extracts the field from the given value
func (p PropertyName[V]) ExtractField(v any) V {
return reflect.ValueOf(v).FieldByName(string(p)).Interface().(V)
}

type keyset[I comparable, V any] struct {
offset int
pageSize int
sqlIdName common.FieldName
idGetter func(V) I
// the first and last id values in the page
firstId, lastId I
}

// KeysetWithField creates a keyset pagination where the id has field name idFieldName
func KeysetWithField[I comparable](offset int, pageSize int, sqlIdName common.FieldName, idFieldName PropertyName[I]) (*keyset[I, any], error) {
if strings.ToUpper(string(idFieldName[0])) != string(idFieldName[0]) {
return nil, fmt.Errorf("must use exported field")
}
return Keyset(offset, pageSize, sqlIdName, idFieldName.ExtractField)
}

type id[I comparable] interface {
Id() I
}

// KeysetWithId creates a keyset pagination where the result object implements id[I]
func KeysetWithId[I comparable, V id[I]](offset int, pageSize int, sqlIdName common.FieldName) (*keyset[I, V], error) {
return Keyset[I, V](offset, pageSize, sqlIdName, func(v V) I { return v.Id() })
}

// Keyset creates a keyset pagination
func Keyset[I comparable, V any](offset int, pageSize int, sqlIdName common.FieldName, idGetter func(V) I) (*keyset[I, V], error) {
if offset < 0 {
return nil, fmt.Errorf("offset must be greater than zero. Offset: %d", offset)
}
if pageSize < 0 {
return nil, fmt.Errorf("page size must be greater than zero. pageSize: %d", pageSize)
}
return &keyset[I, V]{
offset: offset,
pageSize: pageSize,
sqlIdName: sqlIdName,
idGetter: idGetter,
firstId: nilElement[I](),
lastId: nilElement[I](),
}, nil
}

func nilElement[I any]() I {
var zero I
switch any(zero).(type) {
case int:
return any(-1).(I)
case string:
return any("").(I)
default:
panic("unsupported type")
}
}

func (p *keyset[I, V]) nilElement() I {
return nilElement[I]()
}

func (p *keyset[I, V]) GoToOffset(offset int) (driver.Pagination, error) {
if offset < 0 {
return nil, fmt.Errorf("Offset must be greater than zero. pageSize: %d", pageSize)
}
if offset == p.offset+p.pageSize {
return &keyset[I, V]{
offset: offset,
pageSize: p.pageSize,
sqlIdName: p.sqlIdName,
idGetter: p.idGetter,
firstId: p.lastId,
lastId: p.nilElement(),
}, nil
}
return &keyset[I, V]{
Copy link
Contributor

@alexandrosfilios alexandrosfilios Jun 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, here you are handling the case where we call Next() twice (without having fetched the db results first). And I think this is the reason you have to keep offsetoflastid and offsetoffirstid. However, this is not a use case for us. When we call next, we would first fetch the results and then proceed to the next (as all paginations work in UIs). So, instead of taking care of this case for optimized performance, I would just fall back to an offset pagination (you add the new offset to the existing offset and drop the firstId). It may have worse performance, but it is simpler and it is just a fallback for a case that will practically never occur, just so we are protected from unexpected behaviors.

offset: offset,
pageSize: p.pageSize,
sqlIdName: p.sqlIdName,
idGetter: p.idGetter,
firstId: p.nilElement(),
lastId: p.nilElement(),
}, nil
}

func (p *keyset[I, V]) GoToPage(pageNum int) (driver.Pagination, error) {
return p.GoToOffset(pageNum * p.pageSize)
}

func (p *keyset[I, V]) GoForward(numOfpages int) (driver.Pagination, error) {
return p.GoToOffset(p.offset + (numOfpages * p.pageSize))
}

func (p *keyset[I, V]) GoBack(numOfpages int) (driver.Pagination, error) {
return p.GoForward(-1 * numOfpages)
}

func (p *keyset[I, V]) Prev() (driver.Pagination, error) { return p.GoBack(1) }

func (p *keyset[I, V]) Next() (driver.Pagination, error) { return p.GoForward(1) }
Loading