sqlc-dev / sqlc

Generate type-safe code from SQL
https://sqlc.dev
MIT License
12.26k stars 778 forks source link

sqlc.embed doesn't follow alias naming for table #2686

Open sapk opened 1 year ago

sapk commented 1 year ago

Version

1.20.0

What happened?

This prevent to do multiple join with the same table. Like a table making between multiple entries like linking users to users.

With the provided example the resulting struct would look like:

type ListUserRelationRow struct {
    User   User
    User_2 User
}

I would expect it to look like:

type ListUserRelationRow struct {
    Owner    User
    Consumer User
}

I could work on a fix. I just first want to make sure that it would be ok to switch to the format I would expect.

Relevant log output

No response

Database schema

CREATE TABLE users (
  id INT UNSIGNED NOT NULL,
  name VARCHAR(255) NOT NULL,
  PRIMARY KEY (id)
);

CREATE TABLE user_links (
  owner_id INT UNSIGNED NOT NULL,
  consumer_id INT UNSIGNED NOT NULL,
  PRIMARY KEY (owner_id, consumer_id)
);

SQL queries

-- name: ListUserRelation :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;

Configuration

{
  "version": "1",
  "packages": [
    {
      "path": "db",
      "engine": "mysql",
      "schema": "query.sql",
      "queries": "query.sql"
    }
  ]
}

Playground URL

https://play.sqlc.dev/p/f81ff911dc5e18f6398998262c44b25169a2b4685d81c9dfc9669d962f3e26a5

What operating system are you using?

Linux

What database engines are you using?

MySQL

What type of code are you generating?

Go

sapk commented 1 year ago

This issue is maybe a duplicate of #2655. I didn't found it before doing a quick search. Their is one main difference is that in #2655 it is more to support alias at columns where this one is more on naming at table join level.

andrei-dascalu commented 1 year ago

I also opened a duplicate of this as an enhancement: https://github.com/sqlc-dev/sqlc/issues/2719

The idea suggested is slightly different though (eg: put this behind a configuration option as to not make it a breaking change). Also:

sapk commented 1 year ago

Reviewing my suggestion PR, I think it can be set behind a configuration option. I just need to look how to access it in this part of the code.

kyleconroy commented 1 year ago

I think this might be worth a breaking change. As discussed in #2718, using these aliases doesn't work right now. sqlc adopts the alias name of a column as the name of the field, doing that here. @sapk You don't need to update your PR just yet, as I need to verify that this change won't be too large of a breaking change.

sapk commented 1 year ago

Maybe as a safety we could set the option as new logic by default and if someone trigger a corner-cases it can revert to old one if needed. In case of no issue reported, we could remove the config flag to keep only the new logic in later version.

kirk-anchor commented 12 months ago

You can do this on pggen with

-- name: ListUserRelation :many
SELECT
    owner,
    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;

This query executes as-is on Postgres, returning 2 columns of the composite type for the users table. The lack of being able to generate and scan composite types has prevented me from using sqlc. The sqlc.embed() feature is an improvement but does not work with input values such as providing values to use on an INSERT. I need composite types for that and other cases such as in an ARRAY_AGG()

kirk-anchor commented 12 months ago

I see OP is using MySQL so composite types are not an option there. For Postgres users, it would be useful to have a way to scan a composite type of the table to the same struct that sqlc.embed() provides.

andrewmbenton commented 11 months ago

I see OP is using MySQL so composite types are not an option there. For Postgres users, it would be useful to have a way to scan a composite type of the table to the same struct that sqlc.embed() provides.

Are you using pgx/v5? If so I think we'll have a proposal for supporting composite types pretty soon. Here's the placeholder issue to follow: https://github.com/sqlc-dev/sqlc/issues/2760

andrei-dascalu commented 11 months ago

@andrewmbenton @sapk as a mysql user mysql, I'm looking forward to this solution. Will the current PR go ahead as designed?