sqlc icon indicating copy to clipboard operation
sqlc copied to clipboard

sqlc.embed using LEFT JOIN

Open pelletier197 opened this issue 1 year ago • 13 comments

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 avatar Nov 21 '23 14:11 pelletier197

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 avatar Nov 21 '23 22:11 KevenGoncalves

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

KevenGoncalves avatar Nov 21 '23 22:11 KevenGoncalves

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

ltdangle avatar Nov 24 '23 11:11 ltdangle

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

KevenGoncalves avatar Nov 24 '23 11:11 KevenGoncalves

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 avatar Nov 24 '23 11:11 pelletier197

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.

pelletier197 avatar Nov 24 '23 11:11 pelletier197

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 avatar Nov 24 '23 11:11 KevenGoncalves

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

KevenGoncalves avatar Nov 24 '23 11:11 KevenGoncalves

It looks like https://github.com/sqlc-dev/sqlc/pull/2858 fixes this by erroring if a nullable embed is used in a query.

brasic avatar Dec 05 '23 22:12 brasic

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.

hstefan avatar Dec 21 '23 22:12 hstefan

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.

anthonybishopric avatar Jan 11 '24 16:01 anthonybishopric

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

DesnLee avatar Jan 20 '24 19:01 DesnLee

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).

ErikKalkoken avatar Apr 26 '24 10:04 ErikKalkoken