Add emit_result_struct_pointers and emit_params_struct_pointers options#979
Add emit_result_struct_pointers and emit_params_struct_pointers options#979kyleconroy merged 10 commits intosqlc-dev:mainfrom
Conversation
…ns for Go generator
|
Would love to see this landed soon! It'd make the generated types much more idiomatic: https://github.com/golang/go/wiki/CodeReviewComments#receiver-type |
|
@kyleconroy anything we can do to help get this merged? Using pointer types would be a lot more idiomatic imo. |
|
@dchertkov I'm so sorry reviewing this pull request fell through the cracks. If you're willing to fix the merge conflicts and update to the latest code, I can promise to review it as soon as it's ready. Let me know. |
|
@kyleconroy All is ready. |
kyleconroy
left a comment
kyleconroy
left a comment
There was a problem hiding this comment.
Overall this approach looks good. A few things to clean up before we can get this merged.
internal/endtoend/testdata/emit_result_and_params_struct_pointers/query.sql
Show resolved
Hide resolved
| SELECT a, b FROM foo | ||
| ` | ||
|
|
||
| func (q *Queries) GetAll(ctx context.Context) ([]*Foo, error) { |
There was a problem hiding this comment.
The slice of structs seems weird to me, but I guess that's what you'd want in this case?
There was a problem hiding this comment.
There has been some discussion about performance https://stackoverflow.com/questions/27622083/slices-of-structs-vs-slices-of-pointers-to-structs
kyleconroy
left a comment
kyleconroy
left a comment
There was a problem hiding this comment.
We'll need to update the docs with the new configuration options, but that can be done in a separate PR.
3 participants
Add emit_result_struct_pointers and emit_params_struct_pointers options for Go generator.
This is second attempt at improvement as noted here #812