Repository navigation
sqlc.embed() generates code that cannot handle NULL items #2348
Description
Version
1.18.0
What happened?
I was testing to implement sqlc.embed() in order to simplify my conversions routines from sqlc to protobuf.
I have this query:
-- name: GetChannelLayers :many
SELECT sqlc.embed(t)
FROM unnest(sqlc.narg('channel_layer_ids')::TEXT[]) WITH ORDINALITY o(channel_layer_id, ord)
LEFT JOIN channel_layer AS t ON t.channel_layer_id = o.channel_layer_id::UUID AND t.tenant_id = $1;The input is a list of IDs that i expect to get from the DB. If the ID is not found I expect to get a NULL row. Unfortunately it seems like the code for the new sqlc.embed() feature doesn't handle this case very well since I get a scan error if I try to query for a non-existent ID.
pgx.ScanArgError: can't scan into dest[0]: cannot scan null into *string
The generated output code:
const getChannelLayers = `-- name: GetChannelLayers :many
SELECT t.tenant_id, t.channel_id, t.channel_layer_id, t.name, t.description, t.created, t.updated, t.removed, t.disabled, t.condition_ref, t.z_index, t.width, t.height, t.pos_x, t.pos_y, t.alignment, t.content_ref
FROM unnest($2::TEXT[]) WITH ORDINALITY o(channel_layer_id, ord)
LEFT JOIN channel_layer AS t ON t.channel_layer_id = o.channel_layer_id::UUID AND t.tenant_id = $1
`
type GetChannelLayersParams struct {
TenantID string
ChannelLayerIds []string
}
type ChannelLayer struct {
TenantID string
ChannelID uuid.UUID
ChannelLayerID uuid.UUID
Name string
Description sql.NullString
Created time.Time
Updated time.Time
Removed sql.NullTime
Disabled sql.NullTime
ConditionRef sql.NullString
ZIndex sql.NullInt32
Width sql.NullInt32
Height sql.NullInt32
PosX sql.NullInt32
PosY sql.NullInt32
Alignment ChannelLayerAlignment
ContentRef sql.NullString
}
type GetChannelLayersRow struct {
ChannelLayer ChannelLayer
}
func (q *Queries) GetChannelLayers(ctx context.Context, arg GetChannelLayersParams) ([]*GetChannelLayersRow, error) {
rows, err := q.db.Query(ctx, getChannelLayers, arg.TenantID, arg.ChannelLayerIds)
if err != nil {
return nil, err
}
defer rows.Close()
var items []*GetChannelLayersRow
for rows.Next() {
var i GetChannelLayersRow
if err := rows.Scan(
&i.ChannelLayer.TenantID,
&i.ChannelLayer.ChannelID,
&i.ChannelLayer.ChannelLayerID,
&i.ChannelLayer.Name,
&i.ChannelLayer.Description,
&i.ChannelLayer.Created,
&i.ChannelLayer.Updated,
&i.ChannelLayer.Removed,
&i.ChannelLayer.Disabled,
&i.ChannelLayer.ConditionRef,
&i.ChannelLayer.ZIndex,
&i.ChannelLayer.Width,
&i.ChannelLayer.Height,
&i.ChannelLayer.PosX,
&i.ChannelLayer.PosY,
&i.ChannelLayer.Alignment,
&i.ChannelLayer.ContentRef,
); err != nil {
return nil, err
}
items = append(items, &i)
}
if err := rows.Err(); err != nil {
return nil, err
}
return items, nil
}I'm actually not sure how to solve this. I need the LEFT JOIN because reasons...
Perhaps it's impossible to support this case?
Perhaps sqlc can create a temp/hidden type where all fields are NULL-able. Scan the fields using this type and then check if the PRIMARY KEY fields are Valid. If they are valid, convert the type with the NULL-able fields into the normal type. If the fields are not valid then return a nil object pointer for the embedded field.
@nickjackson, any input?
What operating system are you using?
Linux
What database engines are you using?
PostgreSQL
What type of code are you generating?
Go
Activity
bas-ie commentedon Jul 2, 2023
Playground reproduction: https://play.sqlc.dev/p/f7baf03ee43839b8063555ea33f6d94c2f9f8545e14cb7f41406f880eea432d7
I may actually try to fix this, as it also affects my planned use for
embed. Looking at the source, I'm still not quite sure how much work is involved making it respect nullable columns. I'll have a think about it.nickjackson commentedon Jul 2, 2023
Hi Both,
Left joins were left out of the implementation because the scanning logic is not so straightforward. Passing the additional context through about a left-join column is also tricky.
I think most of this idea has legs (temp type with nullable fields, nil object pointer, seem 👍 ), but there are two issues I can see.
Validattribute or none at all? If done incorrectly, the user will end up with a broken package.Both of those options are solvable, perhaps with config overrides, but it's worth pointing out. This feature requires sqlc to make business logic decisions which it typically stays out of.
I would love to see this implemented, just to make
sqlc.embedfeature complete. I might even use it, but right now I just work around this by making an additional query in parallel.I hope this helps.
bas-ie commentedon Jul 3, 2023
Yeah, sometimes it's a question of "where do you stop?" with a feature like this I guess. 🤔
It'd be tempting to explicitly provide the support if the primary key is available, and bail out to the standard behaviour if not. This would surely increase its utility quite a bit. However, I do wonder if there's an opportunity for the generator logic to be a bit more well-informed about which joins could result in a null
embedeven if we're not joining on a primary key (for example, always using anilpointer for a left join).Alternatively, could there be room for a
sqlc.embed(foo, sqlc.nullable)variant? Which would make it declarative, and put the onus on the developer to request it.PatrLind commentedon Jul 3, 2023
Good points nickjackson. I also think richchurcher has a point on the ability to configure the embed() command with parameters.
I think a good start could be to write the code such that it is able to detect the cases it cannot support. Right now the code generated doesn't really work if you happen to do unsupported things. If it is possible to detect all the cases it doesn't work, I think it would be easier to extend it to actually make it work, since then we know all about what type of join it is and so forth.
I took a look at the code, and sqlc doesn't seem to know very much about the join itself, just that there is some kind of relationship to another table.
nickjackson commentedon Jul 3, 2023
Yeah, I have to be honest the implementation (and the whole of sqlc to some extent) is very naive about what a join is - it is just sugar on column selections.
Happy to answer questions and help out where I can, but I don't have the bandwidth to do this myself, nor do I have any authority to suggest the right course of action. (I'm not a maintainer)
bas-ie commentedon Jul 3, 2023
I'll have a quiet look at what it would take to go down the parameter path... it feels like it would just be a question of choosing a pointer type, but it can't possibly be that simple 😆
bas-ie commentedon Jul 9, 2023
I think, having done more poking around, that even if piggybacking on
embedmight be slightly easier, introducingnembed(nullable embed) is more in line with the existence ofarg/narg. From the looks of it, it'll require a bit of a refactor but maybe I can learn from @skabbes' hard work in #1536 .bas-ie commentedon Jul 18, 2023
@nickjackson If you did happen to have a moment, could you glance at #2472 and see if you think a more polished version of this might work?
andrewmbenton commentedon Jul 28, 2023
I think I understand the motivation for this after reading, but based on the current PR it's going to require a lot of changes including changes to templates so I'm not sure if it's likely to get merged.
Just to clarify, would you say the "workaround" for this is to abandon use of
sqlc.embed()? I'm not even sure that aSELECT * ...would work in this case...bas-ie commentedon Aug 31, 2023
Oh I don't think it'll get merged either, but I'm glad to have talked about the issue more :) At the moment, this is my workaround for nullable joins with
embed:Being consistent with that naming scheme lets me copy the values into structs in a relatively pain-free fashion. It's work, but it's not too much work, at least for now.
KevenGoncalves commentedon Sep 14, 2023
Unfortunately even with that I still get the Scan error, so I was forced to write two queries, one with the possible null relations and other without it, but is quite painful
In this case the relations that can be null is transport and manager
bas-ie commentedon Sep 15, 2023
Yeah, you won't be able to use
embedfortande, you need to handle 'em as columns and do some post-processing after the query.hectorj-thetreep commentedon Oct 11, 2023
Workaround: if you add a view with just your
LEFT JOIN, you canJOINit and sqlc will generate the model with all nullable fields. See https://play.sqlc.dev/p/1f63b8bc9aac38f8be54ce6b4842b693ed259cf7a4807feaf55aebd1cc8c886akevlarr commentedon Nov 8, 2023
You don't even need to join inside that view, do you? Ie.
create view images_vw as (select * from images)?Either way that's a clever workaround that I'm probably going to rely on for now, I just wish it didn't introduce a model with all nullable fields in our domain that only technically accommodates for only null or a model with all non-nullable fields. Dreaming of
sqlc.nembed.KevenGoncalves commentedon Nov 8, 2023
Thanks Man, I never thought about that, I didn’t know that a view generate Null fields.
hungtcs commentedon Jul 11, 2024
Hey, I'm stumped by this.
In my code, I have to use left join to query the owner(nullable) of the resource,
In order to avoid errors, I now have to write out all columns in the select with
owner_prefix. but that's a lot, theownertable could have hundreds of columns.Do you have a better way? Thanks.
KevenGoncalves commentedon Jul 11, 2024
You can use views to handle that. see #2997
hungtcs commentedon Jul 11, 2024
@KevenGoncalves
I just saw this answer but didn't understand the principle, now I understand, it looks quite feasible, I just need to check whether the primary key is nil or not. Thanks a lot.
sekulicd commentedon Jul 24, 2024
@KevenGoncalves does workaround with views makes sense with multiple tables?
This is my query:
Here is schema:
sekulicd commentedon Jul 25, 2024
Update:
Bellow worked for me:
Jonathan-Landeed commentedon Feb 12, 2025
Here's a working version that can be done once per table instead of for every left join in your codebase:
The explain analyze output looks the same for this and normal queries, so there shouldn't be much of a performance loss.
The
create view images_vw as (select * from images)version didn't work for me.I started with this
CREATE VIEW images_vw AS SELECT images.* FROM (SELECT NULL) dummy LEFT JOIN images ON TRUE;but it has a slight problem of returning an empty row if you select from it directly and there aren't any images.feat(sqlc-dev#2348, sqlc-dev#2997): add sqlc.nembed() for nullable em…