Improve sqlc.embed Go field naming

[sapk]

An attempt to fix #2686

It make the argument of sqlc.embed used as the name of the field name in Go struct. This allow join of the same table with custom name and not forcing to be incremental naming.

This change can break previous models using sqlc.embed

[sapk]

I added the change under a configuration flag so that it isn't a breaking change by default. If you don't like the config flag I can revert the last commit.

[sapk]

I rebased the PR on latest main and updated the tests results for the new version.

[andrei-dascalu]

@sapk any help needed here?

[sapk]

This PR is ready just let me know if it need any change. The latest version, keep backward compatibility via a flag using by default the old logic.

From discussion in #2686 maybe you want me to remove this flag? or reverse the logic to set it as by default active?

[andrei-dascalu]

@sapk I was asking just because here it says there are conflicts so it can't be merged. Not sure who can approve, maybe @kyleconroy ?

[sapk]

Yes I added a commit after to make it configurable and keep a backward compat by default.

[sapk]

Some change in the struct of the configuration seems to have broken this MR. I will rebase this work on the new configuration logic.

[andrewmbenton]

Some change in the struct of the configuration seems to have broken this MR. I will rebase this work on the new configuration logic.

Yes, sorry about that. We had to change a few things to support publishing sqlc-gen-go. All Go codegen configuration now lives inside of the codegen/golang package. The change means you won't need to change anything in the codegen plugin proto, and can avoid all the changes in config and shim.

[sapk]

No worries. It is now easier to add (or remove) option. The PR should be up-to-date now.

[sapk]

To resume, this PR allow to have returning struct relying on alias in sqlc.embed().:

type ListUserLinkRow struct {
	Owner    User
	Consumer User
}

instead of

type ListUserLinkRow struct {
	User   User
	User_2 User
}

for query like

-- name: ListUserLink :many
SELECT
    sqlc.embed(owner),
    sqlc.embed(consumer)
FROM
    user_links
    INNER JOIN users AS owner ON owner.id = user_links.owner_id
    INNER JOIN users AS consumer ON consumer.id = user_links.consumer_id;

We still get enumerate in case of duplicate even if the option is enabled:

-- name: Duplicate :one
SELECT sqlc.embed(users), sqlc.embed(users) FROM users;

type DuplicateRow struct {
	User   User
	User_2 User
}

[andrewmbenton]

Before getting into code comments, I think @kyleconroy should weigh in on whether this is a good idea and if so whether we should just make this naming behavior the default and remove the option flag, or invert the flag to allow opting back in to the old behavior.

[sapk]

@andrewmbenton I agree, that's why when I rebased the PR, I split it into two commits. One that implement the new logic and one that put it under a flag so that if you don't want the flag it can easily be removed.

[sapk]

I updated the PR from main latest and added missing documentation for the potential new configuration.

[bloeys]

Hey everyone,

Any update about this?
This feature is seriously needed because with the current behavior there is just no way to know what each embedded field represents.

Regarding backward compatibility, why not simply accept a second (optional) parameter that signifies the name?
For example:

SELECT sqlc.embed(table1, 'MyCustomName')

Generates the following struct:

type S struct {
  MyCustomName Table1
}

All code that doesn't pass this second parameter simply follows the current behavior, so we don't break anyone's code.

[sapk]

I updated the PR.

@bloeys I first go as backward compatibility with configuration and try to have a similar logic as named parameters so that the configuration could be removed after transition. But your idea is good and could be an other solution. I could implement it if it is preferred by project owners.