Proposal: Add CLI flag to always generate JSON tags for structs

[farazfazli]

Right now JSON struct tags are only generated for <query_name>Row structs according to the README.

What are your thoughts on adding a CLI flag (such as --json-struct-tags) to always generate JSON tags whenever structs are generated? This would be useful while building REST APIs.

Looking forward to hearing back. Thanks again for the great project!

[jschaf]

Do you mean adding JSON tags to the input structs as well? Is the idea you take JSON that directly matches the input parameters?

[farazfazli]

@jschaf Hey thanks for replying so quickly. Yes exactly, that's what I had in mind. What are your thoughts on this?

[jschaf]

I think adding JSON tags is okay.

Two thoughts:

• Applications shouldn't blindly trust API request data. Making it too easy to create the params struct from JSON could encourage skipping validation.
• Coupling the params struct to a public API makes changing the database structure harder.

Do those thoughts impact your use-case?

[farazfazli]

Glad to hear that. To address your points:

Applications shouldn't blindly trust API request data. Making it too easy to create the params struct from JSON could encourage skipping validation.

Check constraints can be used on the underlying tables for validating things on the database side. I feel validating things is up to the developer to enforce more-so than the tooling making it easier or harder. I do understand your point though.

Coupling the params struct to a public API makes changing the database structure harder.

Views can be created as an abstraction over Postgres tables, hiding the implementation details of the underlying tables from the public API. That way, the tables can evolve over time while the views remain stable.

[farazfazli]

It looks like a few things are needed to get this to work:

1. New flag is added to newGenCmd() (perhaps we can call it --always-emit-json-tags, open to suggestions)
2. New boolean value is created as part of TemplatedQuery / passed to NewTemplater
3. We check the boolean value inside EmitParamStruct(), if true add a sb.WriteString call to write the JSON tag alongside the other generated code.

Is this approach correct? If so, happy to open a PR with these changes. Would just need your help to get some tests written for this functionality. Thank you. :)

[jschaf]

1. I'd prefer to always emit the JSON struct tags to avoid the flag. Seems harmless enough. Technically, it's a breaking change if someone already depends on the default Go tags, but I think it's worth it to avoid the flag (one of the design goals is minimal API surface, which includes flags).

2. Can skip it since it's unconditional.

3. Also skippable.

The interesting decision is what to use for the JSON tag name. Options are:

1. The raw pggen name. I'm leaning toward this option since it's the only customizable option.
2. The UpperCamelCase name. Pggen generates this to use for the Param field name
3. The lowerCamelCase name. pggen generates when scanning variables from Postgres. It's probably the closest to the "standard" convention for JSON keys.

Any preference?

The main change would be in:

https://github.com/jschaf/pggen/blob/fe0c28d62d37d84ccd7f2cf48ff34b3a8354b1dc/internal/codegen/golang/templated_file.go#L110

And look very similar to:

https://github.com/jschaf/pggen/blob/fe0c28d62d37d84ccd7f2cf48ff34b3a8354b1dc/internal/codegen/golang/templated_file.go#L472

I'm happy to talk you through it if you want to send a PR.

[farazfazli]

Ok - I made it emit by default. Regarding the options, I think the first option is the best as this is customizable which is useful to the user.

I opened a draft PR, please see #90. Interested in hearing your feedback.