Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Have the generator use existing structs as inputs/outputs for generated functions #2625

Unanswered
DavidS-ovm asked this question in Feature Requests & Ideas
Discussion options

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 implementation
func (h *ApiKeyServiceHandler) GetAPIKey(ctx context.Context, req *connect.Request[sdp.GetAPIKeyRequest]) (*connect.Response[sdp.GetAPIKeyResponse], error) {
	// skipped authorization code
	databaseKey, err := h.Queries.GetApiKeyByID(ctx, models.GetApiKeyByIDParams{
		ExternalID: id,
		AccountName: claims.AccountName,
	})
	// skipped error handling
	resp := connect.NewResponse(&sdp.GetAPIKeyResponse{
		Key: databaseKey.ToSDP(),
	})
	return resp, nil
}
// ApiKey is the generated model from sqlc
func (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,
		},
	}
	if k.Error.Valid {
		result.Metadata.Error = k.Error.String
	}
	return result
}
-- name: GetApiKeyByID :one
SELECT *
FROM api_keys
WHERE account_name = @account_name
 and external_id = @external_id;

instead it would be really nice to be able to say

-- name: GetApiKeyByID :one :go:result:*sdp.APIKey
SELECT *
FROM api_keys
WHERE account_name = @account_name
 and external_id = @external_id;

and boil the handler down to

// current implementation
func (h *ApiKeyServiceHandler) GetAPIKey(ctx context.Context, req *connect.Request[sdp.GetAPIKeyRequest]) (*connect.Response[sdp.GetAPIKeyResponse], error) {
	// skipped authorization code
	databaseKey, err := h.Queries.GetApiKeyByID(ctx, models.GetApiKeyByIDParams{
		ExternalID: id,
		AccountName: claims.AccountName,
	})
	// skipped error handling
	resp := connect.NewResponse(&sdp.GetAPIKeyResponse{
		Key: databaseKey // pass object through directly
	})
	return resp, 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, arg GetApiKeyByIDParams) (*sdp.APIKey, error) {
	row := q.db.QueryRowContext(ctx, getApiKeyByID, arg.AccountName, arg.ExternalID)
	var externalId uuid.Uuid
	var accountName string
	// etc
	err := row.Scan(
		&externalID,
		&accountName,
		// etc
	)
	var i sdp.APIKey
	i.ExternalID = externalId
	i.AccountName = accountName
	// etc
	return &i, err
}
You must be logged in to vote

Replies: 1 comment 4 replies

Comment options

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.

You must be logged in to vote
4 replies
Comment options

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. sql.NullString and it's not obvious how to convert that into a proto message field with type string. Or maybe I'm misunderstanding completely?

Comment options

ach, apologies. I left out the most important bit: timestamppb (the protobuf timestamp implementation) are in this class of non-copyable structs. Thus, having a non-pointer (i.e. what I would want for a mandatory timestamp, like created_at) timestamppb as field in a struct leads to the warnings (and hence the bugs) described in what I linked above.

Comment options

I think I am following now, except for the "using protobuf structs would fail hard" part (emphasis added obv). While it may not be ideal to have a pointer type for e.g. created_at it seems not too big an issue if you get the benefit of reusing your proto structs. But I still may not be understanding the nuance of this, so please correct me if necessary.

Comment options

You're right, I'm being overly negative here. The structs generated by protoc will always have pointers for this, so as long as row.Scan() can deal with that it's gonna work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

AltStyle によって変換されたページ (->オリジナル) /