Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
8 changes: 8 additions & 0 deletions contracts/database/orm/orm.go
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add the same functions in db.go?

Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@ type Query interface {
Begin() (Query, error)
// BeginTransaction begins a new transaction
BeginTransaction() (Query, error)
// Chunk processes a given number of records in batches.
Chunk(count int, callback func([]any) error) error
// Chunk the results of a query by comparing numeric IDs.
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

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

The documentation comment is incorrect. It should describe the ChunkByID method, not be a duplicate of the Chunk comment.

The comment should indicate that ChunkByID chunks results by comparing numeric IDs in ascending order, which avoids issues with offset-based pagination when records are added/deleted during processing.

Suggested change
// Chunk the results of a query by comparing numeric IDs.
// ChunkByID processes records in batches by comparing numeric IDs in ascending order.
// This avoids issues with offset-based pagination when records are added or deleted during processing.

Copilot uses AI. Check for mistakes.
ChunkByID(count int, callback func([]any) error) error
// ChunkByIDDesc processes a given number of records in batches, ordered by ID in descending order.
ChunkByIDDesc(count int, callback func([]any) error) error
Comment on lines +50 to +52
Copy link
Contributor

Choose a reason for hiding this comment

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

The default column is id.

Suggested change
ChunkByID(count int, callback func([]any) error) error
// ChunkByIDDesc processes a given number of records in batches, ordered by ID in descending order.
ChunkByIDDesc(count int, callback func([]any) error) error
ChunkByID(count int, callback func([]any) error, column ...string) error
// ChunkByIDDesc processes a given number of records in batches, ordered by ID in descending order.
ChunkByIDDesc(count int, callback func([]any) error, column ...string) error

// Commit commits the changes in a transaction.
Commit() error
// Count retrieve the "count" result of the query.
Expand Down Expand Up @@ -120,6 +126,8 @@ type Query interface {
OrderByDesc(column string) Query
// OrderByRaw specifies the order should be raw.
OrderByRaw(raw string) Query
// OrderedChunkByID processes a given number of records in batches, ordered by ID.
OrderedChunkByID(count int, callback func([]any) error, descending bool) error
Copy link
Contributor

Choose a reason for hiding this comment

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

It's unnecessary to expose this function.

Copy link
Author

Choose a reason for hiding this comment

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

The functionality is indeed achieved by the other two added methods. I exposed it since it existed in the Laravel interface, but I'm fine with keeping it private. (changed)

// OrWhere add an "or where" clause to the query.
OrWhere(query any, args ...any) Query
// OrWhereBetween adds an "or where column between x and y" clause to the query.
Expand Down
161 changes: 161 additions & 0 deletions database/gorm/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,167 @@ func (r *Query) Cursor() chan contractsdb.Row {
return cursorChan
}

func (r *Query) Chunk(count int, callback func([]any) error) error {
if count <= 0 {
return errors.OrmQueryChunkZeroOrLess
}

var remaining *int
if r.conditions.limit != nil {
limit := *r.conditions.limit
remaining = &limit
}

query := r.addGlobalScopes().buildConditions()

destType := reflect.TypeOf(r.conditions.model)
sliceType := reflect.SliceOf(reflect.PointerTo(destType))
offset := 0

for remaining == nil || *remaining > 0 {
chunkLimit := count
if remaining != nil && *remaining < count {
chunkLimit = *remaining
}

results := reflect.New(sliceType).Interface()

chunkQuery := query.Offset(offset).Limit(chunkLimit).(*Query)
err := chunkQuery.Find(results)
if err != nil {
return err
}

resultsValue := reflect.ValueOf(results).Elem()
length := resultsValue.Len()
if length == 0 {
return nil
}

if remaining != nil {
*remaining = max(*remaining-length, 0)
}

values := make([]any, length)
for i := 0; i < length; i++ {
values[i] = resultsValue.Index(i).Interface()
}

if err = callback(values); err != nil {
return err
}

if length < chunkLimit {
return nil
}

offset += chunkLimit
}

return nil
}
Comment on lines 192 to 258
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

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

The new chunking methods (Chunk, ChunkByID, ChunkByIDDesc, and OrderedChunkByID) lack test coverage. Given that the repository includes comprehensive test coverage for other query methods (e.g., in database/gorm/query_test.go), these new methods should also have corresponding tests to verify:

  • Correct chunking behavior with various count values
  • Proper handling of offset and limit constraints
  • Error handling for invalid count values (count <= 0)
  • Callback error propagation
  • Correct ordering behavior for ChunkByID variants
  • Edge cases like empty results or results smaller than chunk size

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

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

+1, and please add test cases in tests/query_test.go as well.


func (r *Query) ChunkByID(count int, callback func([]any) error) error {
return r.OrderedChunkByID(count, callback, false)
}

func (r *Query) ChunkByIDDesc(count int, callback func([]any) error) error {
return r.OrderedChunkByID(count, callback, true)
}

func (r *Query) OrderedChunkByID(count int, callback func([]any) error, descending bool) error {
if count <= 0 {
return errors.OrmQueryChunkZeroOrLess
}

column := "id"
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

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

The OrderedChunkByID method hardcodes the primary key column name as "id" (line 265), which assumes all models use this column name. This could cause issues when:

  1. Models use a different primary key column name (e.g., user_id, uuid)
  2. The database column name differs from the struct field name due to GORM's column: tag

While database.GetID can retrieve the primary key value using reflection and GORM tags, there's no corresponding mechanism to get the actual database column name for use in the WHERE clauses (lines 305, 307) and ORDER BY clauses (lines 298, 300).

Consider either:

  • Adding a parameter to allow users to specify a custom column name (similar to Laravel's implementation)
  • Creating a utility function to extract the primary key column name from GORM model metadata
  • Documenting this limitation clearly in the method's documentation

Copilot uses AI. Check for mistakes.
initialOffset := 0
if r.conditions.offset != nil {
initialOffset = *r.conditions.offset
}
var remaining *int
if r.conditions.limit != nil {
limit := *r.conditions.limit
remaining = &limit
}

destType := reflect.TypeOf(r.conditions.model)
sliceType := reflect.SliceOf(reflect.PointerTo(destType))
var lastID any
page := 1

for remaining == nil || *remaining > 0 {
chunkLimit := count
if remaining != nil && *remaining < count {
chunkLimit = *remaining
}

clone := r.addGlobalScopes()

if initialOffset > 0 {
if page > 1 {
clone = clone.Offset(0).(*Query)
} else {
clone = clone.Offset(initialOffset).(*Query)
}
}

if descending {
clone = clone.OrderByDesc(column).(*Query)
} else {
clone = clone.OrderBy(column).(*Query)
}

if lastID != nil {
if descending {
clone = clone.Where(column+" < ?", lastID).(*Query)
} else {
clone = clone.Where(column+" > ?", lastID).(*Query)
}
}
clone = clone.Limit(chunkLimit).(*Query)

query := clone.buildConditions()
results := reflect.New(sliceType).Interface()

err := query.Find(results)
if err != nil {
return err
}

resultsValue := reflect.ValueOf(results).Elem()
length := resultsValue.Len()

if length == 0 {
break
}

if remaining != nil {
*remaining = max(*remaining-length, 0)
}

values := make([]any, length)
for i := 0; i < length; i++ {
values[i] = resultsValue.Index(i).Interface()
}

lastRecord := values[length-1]
lastID = database.GetID(lastRecord)
Copy link
Contributor

Choose a reason for hiding this comment

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

lastID may be nil


if err = callback(values); err != nil {
return err
}

if length < chunkLimit {
break
}

page++
}

return nil
}

func (r *Query) DB() (*sql.DB, error) {
return r.instance.DB()
}
Expand Down
1 change: 1 addition & 0 deletions errors/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ var (
OrmMissingWhereClause = New("WHERE conditions required")
OrmNoDialectorsFound = New("no dialectors found")
OrmQueryAssociationsConflict = New("cannot set orm.Associations and other fields at the same time")
OrmQueryChunkZeroOrLess = New("chunk count must be greater than 0")
OrmQueryConditionRequired = New("query condition is required")
OrmQueryEmptyId = New("id can't be empty")
OrmQueryEmptyRelation = New("relation can't be empty")
Expand Down
Loading
Loading