- Notifications
You must be signed in to change notification settings - Fork976
Have the generator use existing structs as inputs/outputs for generated functions#2625
-
in the continued quest to crisp up my APIs, I was wondering what the thoughts on here are regarding specifying custom structs for inputs/outputs of the generated query functions. Specifically in my case I have existing structs generated from the protobuf definition of my API that in some cases would be very convenient to directly (pre-)fill from the database layer: // current implementationfunc (h*ApiKeyServiceHandler)GetAPIKey(ctx context.Context,req*connect.Request[sdp.GetAPIKeyRequest]) (*connect.Response[sdp.GetAPIKeyResponse],error) {// skipped authorization codedatabaseKey,err:=h.Queries.GetApiKeyByID(ctx, models.GetApiKeyByIDParams{ExternalID:id,AccountName:claims.AccountName,})// skipped error handlingresp:=connect.NewResponse(&sdp.GetAPIKeyResponse{Key:databaseKey.ToSDP(),})returnresp,nil}// ApiKey is the generated model from sqlcfunc (k*ApiKey)ToSDP()*sdp.APIKey {result:=&sdp.APIKey{Metadata:&sdp.APIKeyMetadata{Uuid:k.ExternalID[:],Created:&k.CreatedAt,LastUsed:k.LastUsed,Key:k.Key,Scopes:k.Scopes,Status:sdp.KeyStatus(k.Status),},Properties:&sdp.APIKeyProperties{Name:k.Name,},}ifk.Error.Valid {result.Metadata.Error=k.Error.String}returnresult} -- name: GetApiKeyByID :oneSELECT*FROM api_keysWHERE account_name= @account_nameand external_id= @external_id; instead it would be really nice to be able to say -- name: GetApiKeyByID :one :go:result:*sdp.APIKeySELECT*FROM api_keysWHERE account_name= @account_nameand external_id= @external_id; and boil the handler down to // current implementationfunc (h*ApiKeyServiceHandler)GetAPIKey(ctx context.Context,req*connect.Request[sdp.GetAPIKeyRequest]) (*connect.Response[sdp.GetAPIKeyResponse],error) {// skipped authorization codedatabaseKey,err:=h.Queries.GetApiKeyByID(ctx, models.GetApiKeyByIDParams{ExternalID:id,AccountName:claims.AccountName,})// skipped error handlingresp:=connect.NewResponse(&sdp.GetAPIKeyResponse{Key:databaseKey// pass object through directly})returnresp,nil}// no ApiKey model generated, no manual copying required Of course all the property names and mappings would have to align and all properties would need to have compatible enough types, which could be checked by generating intermediate temp variables in the generated functions like this: func (q*Queries)GetApiKeyByID(ctx context.Context,argGetApiKeyByIDParams) (*sdp.APIKey,error) {row:=q.db.QueryRowContext(ctx,getApiKeyByID,arg.AccountName,arg.ExternalID)varexternalId uuid.UuidvaraccountNamestring// etcerr:=row.Scan(&externalID,&accountName,// etc)vari sdp.APIKeyi.ExternalID=externalIdi.AccountName=accountName// etcreturn&i,err} |
BetaWas this translation helpful?Give feedback.
All reactions
Replies: 1 comment 4 replies
-
I just realized that my primary use-case would most likely not work for reasons outside of sqlc: using protobuf structs would fail hard in practice because protobuf in general doesn't allow shallow copies (see e.g.https://stackoverflow.com/a/67796278/4918 for an explanation) and would break horribly for not-nullable fields or add complexity dealing with nil-able pointers to those structs even when there would be a guaranteed result. |
BetaWas this translation helpful?Give feedback.
All reactions
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
-
Why wouldn't this work in a case where a database field is not-nullable? It seems like the opposite would be true, i.e. if you have a nullable field then you'd typically get back e.g. |
BetaWas this translation helpful?Give feedback.
All reactions
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
-
ach, apologies. I left out the most important bit: |
BetaWas this translation helpful?Give feedback.
All reactions
-
I think I am following now, except for the "using protobuf structs wouldfail hard" part (emphasis added obv). While it may not be ideal to have a pointer type for e.g. |
BetaWas this translation helpful?Give feedback.
All reactions
-
You're right, I'm being overly negative here. The structs generated by protoc will always have pointers for this, so as long as |
BetaWas this translation helpful?Give feedback.