Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sqlc.embed using LEFT JOIN #2997

Open
pelletier197 opened this issue Nov 21, 2023 · 13 comments
Open

sqlc.embed using LEFT JOIN #2997

pelletier197 opened this issue Nov 21, 2023 · 13 comments
Labels
bug Something isn't working triage New issues that hasn't been reviewed

Comments

@pelletier197
Copy link

pelletier197 commented Nov 21, 2023

Version

1.23.0

What happened?

When using sqlc.embed on something in combination with a LEFT JOIN, meaning that the embedded table may be null, sqlc fails to parse the result when null. This happened with SQLite, but considering the nature of the issue, this might be relevant for other databases.

Relevant log output

sql: Scan error on column index 1, name "id": converting NULL to int64 is unsupported

Database schema

CREATE TABLE accounts(
    id integer NOT NULL PRIMARY KEY AUTOINCREMENT
);

CREATE TABLE orders(
    id integer NOT NULL PRIMARY KEY AUTOINCREMENT
);

CREATE TABLE account_orders(
                               account_id integer NOT NULL REFERENCES accounts(id),
                               order_id integer NOT NULL REFERENCES orders(id)
);

INSERT INTO accounts (id) VALUES (1);

SQL queries

-- name: GetAllAccounts :many
SELECT accounts.id, sqlc.embed(orders)
from accounts
         LEFT JOIN account_orders ON accounts.id = account_orders.account_id
         LEFT JOIN orders ON orders.id = account_orders.order_id;

Configuration

# Although irrelevant for this issue I believe

version: "2"
sql:
  - engine: "sqlite"
    queries: "infrastructure/sqlite/sqlc/queries"
    schema: "infrastructure/sqlite/sqlc/schema.sql"
    gen:
      go:
        package: "dao"
        out: "infrastructure/sqlite/sqlc/dao"
        emit_interface: true

Playground URL

No response

What operating system are you using?

Linux

What database engines are you using?

SQLite

What type of code are you generating?

Go

@pelletier197 pelletier197 added bug Something isn't working triage New issues that hasn't been reviewed labels Nov 21, 2023
@KevenGoncalves
Copy link

Seems like you query have some problems but if is not, This is an old problem with sqlc using embed, it cannot generate pointer types for tables, only the normal types, and what is happening is the scan is trying to pass null to non-null type and give that error, there's two workarounds that you can implement, the first one is take care of possibility of null values to be passed to the scan, sometimes that is not viable and the second workaround and most valuable is create a view with a join for that table, so you can pass instead the table, pass the view, because the sqlc generate views with null types check here

@KevenGoncalves
Copy link

Try query without sqlc.embed in SQLite client, to check the nullity of some fields

@ltdangle
Copy link

Can confirm the same issue for mysql target.
I guess the solution is not to use sqlc.embed...

@KevenGoncalves
Copy link

Can confirm the same issue for mysql target. I guess the solution is not to use sqlc.embed...

You can try use the view solution

@pelletier197
Copy link
Author

pelletier197 commented Nov 24, 2023

Yes, that's what I did. It's just quite inconvenient because I have multiple field id and name in my real tables, so sqlc ends up generating id_1,id_2... fields. I had to manually select each and every field of my entity and name them properly.

In the end, this is an annoyance and it does not allow me to re-use my assembler code, but if you feel like there is no way to fix this, then you may just close the issue.

However, if there is a potential fix, I think this would greatly benefit everyone. Thanks for your help 🙂

@pelletier197
Copy link
Author

pelletier197 commented Nov 24, 2023

As a potential other idea, would it be easier for sqlc to avoid naming things id_1 and Id_2 when I do select accounts.*, orders.*? If it could throw a prefix in there that could be great account_id, order_id, and so on.

I'm just throwing the idea.

@KevenGoncalves
Copy link

Yes, that's what I did. It's just quite inconvenient because I have multiple field id and name in my real tables, so sqlc ends up generating id_1,id_2... fields. I had to manually select each and every field of my entity and name them properly.

In the end, this is an annoyance and it does not allow me to re-use my assembler code, but if you feel like there is no way to fix this, then you may just close the issue.

However, if there is a potential fix, I think this would greatly benefit everyone. Thanks for your help 🙂

Yeah I Think the same if could have some thing like sqlc.nembed() that generate the table with null values would be awesome, the only way right now is use the view and have duplicate structs

@KevenGoncalves
Copy link

As a potential other idea, would it be easier for sqlc to avoid naming things id_1 and Id_2 when I do select accounts.*, orders.*? If it could throw a prefix in there that could be great account_id, order_id, and so on.

I'm just throwing the idea.

The problem is that the sqlc "cannot" see that the id is from a certain table or not, but if i'm not committing a mistake if you use the enhanced queries from the new version that can be solved

@brasic
Copy link

brasic commented Dec 5, 2023

It looks like #2858 fixes this by erroring if a nullable embed is used in a query.

@hstefan
Copy link

hstefan commented Dec 21, 2023

Ran into this issue just now.

Would it make sense to extend sqlc.embed to use pointers for structs extracted from left joins? That's not as neat as sql.Null* types, but it's still workable from a user perspective. The hacky part would be determining that all fields are null, I believe.

@anthonybishopric
Copy link

Also running into this. It's a bit of a pain as the path of least resistance to fixing this is breaking out multiple queries and doing the stitching in code myself. This is the worst issue I've run into with sqlc thus far - it's easier to write multiple queries and hurt site performance than any other option.

Agreed with @hstefan - an alternative to a pointer type could be to use a generic Optional[T] when left joining instead of just embedding the original type, or generating an OptionalThing for every Thing that's generated.

@DesnLee
Copy link

DesnLee commented Jan 20, 2024

I guess the best solution for me now is to split the query and then combine it manually in the code?

@ErikKalkoken
Copy link

Splitting the query works, but if one is looking for a solution that also performs well in lists, then I think creating views as suggested above by @KevenGoncalves is better. The views can be used in a left join (as requested by OP), because all fields of the generated view models are nullable. The view models still need to be mapped back to the source models. But that is just some go mapping and better then running additional queries for each object (see also n+1 problem).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triage New issues that hasn't been reviewed
Projects
None yet
Development

No branches or pull requests

8 participants