-
Notifications
You must be signed in to change notification settings - Fork 880
WithTx is not part of the method set of interface generated #383
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
Comments
I actually don't think it's possible to include the There may be a better way to do this. I'll need to think about it a bit more. |
I guess most project which will adoptsqlc, will try to keep flexibility for some custom methods which will not come under code genration. Then interface embedding will be required. |
Hi @kyleconroy , what do you suggest meanwhile to handle transactions. |
I have a similar problem. I wanted to add a BeginTx call to my app, but I was using the interface everywhere, so I would have to tear all that out and replace it with a concrete type to do that. I think it would make sense to add |
Can't you just create a wrapper interface and struct? package wrapper
type Querier interface {
db.Querier
WithTx(tx *sqlx.Tx) Querier
}
type Queries struct {
*db.Queries
}
func (q *Queries) WithTx(tx *sqlx.Tx) Querier {
return &Queries{
Queries: q.Queries.WithTx(tx.Tx),
}
}
func NewQuerier(q *db.Queries) Querier {
return &Queries{
Queries: q,
}
} I haven't tested this out yet; I'll edit this post if there's any problems with this approach. |
Hi there! I ran into related issues recently and put up #1279 to try to address them. I'd appreciate any thoughts and feedback you might have. |
@ScottyFillups your solution has worked well for me. It seems nice to be able to fold the database connection inside the Querier, so that there's only one thing to pass around. I tried to implement what @carlmjohnson was asking for but it's complicated to get right, I think. So in order to be able to avoid passing sql.DB all over my code, I extended Scotty's solution in what is admittedly a clunky way:
I store the DB in the
The escape hatch of being able to get the DB() out when you need it in order to start a transaction is really helpful.
Minor Update: this worked well for me until I needed to mock this interface, and wound up calling into something which in turn called DB(). Something to consider. |
What was the problem with that? I just implemented something similar but I don't see what the issue would be with mocking. |
I've just ran into this problem as well, appreciate if the interface for querier could be fixed so we can rely on interface in declaration rather than concrete queries |
I'm running into this too.. and the wrapper work around doesn't work for me.. am I doing something wrong? type Querier interface {
db.Querier
WithTx(tx *sql.Tx) Querier
} I get the error
If I instead use |
The new WithTx method needs to have another name as Go doesn't support overriding embedded methods, e.g. // QuerierX is an interface that extends the generated Querier interface with methods that return an interface instead of a *Queries.
type QuerierX interface {
Querier
WithTransaction(tx pgx.Tx) QuerierX
}
// WithTransaction is a wrapper around WithTx that returns an interface instead of a *Queries.
func (q *Queries) WithTransaction(tx pgx.Tx) QuerierX {
return q.WithTx(tx)
} |
Hi, I have interface embedding where my db interface is embedding sqlc generated interface. As db interface is then injected to service layer, I am not able to use this. Is it possible to include WithTX merhod in emitted interface also.
The text was updated successfully, but these errors were encountered: