Skip to content

Conversation

@Jille
Copy link
Contributor

@Jille Jille commented Aug 14, 2023

sqlc's generated code doesn't use that method (unless emit_prepared_queries is enabled). Let's exclude it from the generated interface so that people providing a custom DBTX don't have to implement a unused function.

This is a minor backwards compatibility break, if someone used the generated DBTX for other code that did actually call PrepareContext.

@Jille Jille force-pushed the no-preparecontext branch 2 times, most recently from be5d3e1 to 394bc74 Compare August 14, 2023 20:12
sqlc's generated code doesn't use that method (unless
emit_prepared_queries is enabled). Let's exclude it from the generated
interface so that people providing a custom DBTX don't have to implement
a unused function.

This is a minor backwards compatibility break, if someone used the
generated DBTX for other code that did actually call PrepareContext.
@Jille Jille force-pushed the no-preparecontext branch from 394bc74 to ce902f1 Compare August 14, 2023 20:28
@kyleconroy
Copy link
Collaborator

I agree that this is what we should have done at the start, but now I think it's too late to make this change. The interface is exported in the package, so other code may be depending on the PrepareContext method being on the interface.

@Jille
Copy link
Contributor Author

Jille commented Aug 29, 2023

This would be a relatively minor breakage for very few people, right? The vast majority of users wouldn't be affected. The only way someone would be affected is if they added more files inside the generated package that would use PrepareContext on Queries.db, without actually using sqlc's prepared queries support.

Previous releases have also broken backwards compatibility (for example #1746) and the many other changes that improve the choice of generated types.

My vote would be to make this change today rather than possibly deciding we want it later when we have 10x the number of users.

@kyleconroy
Copy link
Collaborator

The only way someone would be affected is if they added more files inside the generated package that would use PrepareContext on Queries.db, without actually using sqlc's prepared queries support.

The DBTX interface is exported, so any other package could be relying on that method.

@andrewmbenton
Copy link
Collaborator

I think this is a good case for forking and modifying the sqlc-gen-go plugin. We consider this to be too big of a breaking change to merge.

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.

3 participants