Skip to content
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

feat: Enhance Apply method to accept multiple functions #1049

Merged
merged 1 commit into from
Nov 8, 2024

Conversation

Tiscs
Copy link
Collaborator

@Tiscs Tiscs commented Nov 7, 2024

This pull request includes updates to the Apply method across multiple query types to support variadic function arguments, allowing multiple functions to be applied in sequence, these changes will not break existing calls.

type UsersRepo interface {
    // Implemention:
    //
    // var users []*models.User
    // query := r.bdb.NewSelect().Model(&users).Apply(fns...)
    // return query.ScanAndCount(ctx)
    //
    // Example 1:
    //
    //   // find users 
    //   users, _, err := usersRepo.FindUsers(ctx,
    //       func(sq *bun.SelectQuery) *bun.SelectQuery {
    //           return sq.Where("age > ?", 59).Order("age DESC")
    //       },
    //       func(sq *bun.SelectQuery) *bun.SelectQuery {
    //           return sq.Offset(0).Limit(100)
    //       },
    //   )
    FindUsers(ctx context.Context, fns ...func(*bun.SelectQuery) *bun.SelectQuery) ([]*models.User, int, error)
}

BTW, some comments were fixed.

@j2gg0s j2gg0s self-requested a review November 7, 2024 08:00
@j2gg0s
Copy link
Collaborator

j2gg0s commented Nov 7, 2024

Thanks for your contribution!

However, I think Query is chainable, so could we achieve the same effect with the following approach?

db.NewSelect().Model(new(User)).Apply(fn1).Apply(fn2)

@j2gg0s j2gg0s self-assigned this Nov 7, 2024
@Tiscs
Copy link
Collaborator Author

Tiscs commented Nov 7, 2024

Thanks for your contribution!

However, I think Query is chainable, so could we achieve the same effect with the following approach?

db.NewSelect().Model(new(User)).Apply(fn1).Apply(fn2)

Thanks for your response.

That's right, the query can be implemented in your way.

But in my case, I build the query by passing in variadic parameters, so I need to do it like this:

q := db.NewSelect().Model(new(User))
for _, fn := range fns {
    fn(q) // or `q = fn(q)`, queries are modified in-place
}
q.Scan(ctx)

So, I think chainable queries are great, and applying multiple at once is also useful.

query_column_add.go Outdated Show resolved Hide resolved
@Tiscs Tiscs force-pushed the allow_multiple_apply branch from 88a1677 to 46bd9c1 Compare November 7, 2024 09:43
query_column_add.go Outdated Show resolved Hide resolved
@Tiscs Tiscs force-pushed the allow_multiple_apply branch from 46bd9c1 to 028bad6 Compare November 8, 2024 00:37
@Tiscs Tiscs force-pushed the allow_multiple_apply branch from 028bad6 to 7823f2f Compare November 8, 2024 00:45
@j2gg0s j2gg0s merged commit bf96f44 into uptrace:master Nov 8, 2024
4 checks passed
@Tiscs Tiscs deleted the allow_multiple_apply branch November 11, 2024 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants