entc/gen: add support for edge-fields (foreign-keys)

[a8m]

Update

Please see https://entgo.io/docs/schema-edges/#edge-field for full docs.

---

Add an API for loading and getting the FKs of ent models, and gate it with feature-flag.

This change adds a new method on each <T>Query (with FKs on its table) named WithFK() for loading the FKs together with all other fields. However, if users want to select specific fields, they can use Select(...) for selecting any field (including FK). For example:

pets, err = client.Pet.Query().
	WithFKs().
	All(ctx)

pets, err = client.Pet.Query().
	Select(pet.FieldName, pet.ForeignKeys...).
	All(ctx)

Also, for each field-edge (an edge with FK), we generate a new method (e.g. OwnerID() (int, error)), that returns the value of the foreign-key, or an error if it wasn't loaded on query or wasn't found (was NULL in the database). For example:

owner, err := luna.OwnerID()

Next steps (in future PRs) are to add the Select option also to mutations (e.g. Pet.Update()) and predicates for simplifying queries:

pets, err := client.Pet.Query().
	Where(pet.OwnerID(id)).
	All(ctx)

Please share you feedback. Thanks
cc @aight8 @errorhandler @Siceberg @rubensayshi

[a8m]

@marwan-at-work @tmc @cliedeman - I'd love to get your feedback here as well. Thanks 🙏

[rubensayshi]

is there a reason why you don't always expose it? in your pet example the ownerID column is loaded anyway right because pet owns that FK?
so the WithFK is for consistency with M2M or M2O where you need to query for the FKs?

tbh for me the reason for asking for this feature was purely for when the FK is owned by the entity, like the pet, because I don't want to incur another subquery just for the FK.
for M2M and M2O I'm fine with loading the whole edge instead of just the FK.

I don't think it's neccesary to have a featureflag for this, seems like a good default feature, or you just want to keep it experimental for a while to have the option to change it a bit more?

code looks good ;)

[a8m]

is there a reason why you don't always expose it? in your pet example the ownerID column is loaded anyway right because pet owns that FK?
so the WithFK is for consistency with M2M or M2O where you need to query for the FKs?

Thanks for the feedback @rubensayshi.
Regarding your first point - FKs are not loaded by default today, and you need ask for them manually using Select. After this change we expose the edge-fields only for foreign-keys. M2M and M2O require another query to the database and eager-loading already solve this.

I don't think it's neccesary to have a featureflag for this, seems like a good default feature, or you just want to keep it experimental for a while to have the option to change it a bit more?

I usually try adding such things with feature-flags to keep the codegen small as possible (I plan to move more parts to feature-flag in the future), but this also give me the flexibility to release some feature to the OSS, get feedback about the API and improve it if it's needed.

code looks good ;)

Thanks 😃

[marwan-at-work]

I think ideally if Ent allowed us to specify a foreign key in the Fields() schema (note it already works if the edge and the field have two different names), then we wouldn't need this PR and we already would get the "future PRs" @a8m mentioned such as mutations.

Otherwise, I'm happy with whatever everyone thinks is best 👍🏼

[rubensayshi]

hmm okay, I think that it should always select the FKs so you don't need to WithFK, specially since it's "free" and instead of id, err := p.OwnerID() you can just id = p.OwnerID

I already got a PR ready with the .Where(pet.OwnerID(id)) ;) #1192

[adayNU]

I'm assuming this is a replacement of #1046 ?

Agree with @marwan-at-work, having the FK IDs as int (or *int, depending on the edge) fields feels more appropriate. However, if that's not a viable option, I do think having them loaded by default (per @rubensayshi) sounds nice (and avoids the need to handle any error cases).

But worst case, the current state of this PR would still be a big step change improvement :) thanks for your work on this @a8m.

[adayNU]

Let me rephrase, I think I just agree with the original proposal that we should export the FK fields if this flag is turned on (and as is done in the original implementation of this feature here). I'm slightly indifferent on loading them by default (slightly lean to preferring that) but think a WithFKs() call seems perfectly sane.

Or alternatively, would exporting FK fields make sense as a separate feature flag?

[adayNU]

@a8m / @marwan-at-work here's an idea for how to expose the FKs by explicitly defining them as fields - i think these two changes can actually work together. Let me know if you all think this makes sense, if so, I'll continue work on this and open a PR: master...adayNU:idea/fks-of-table

The gist is that you can define a new field type ForeignKey like so:

field.ForeignKey("group_info_id").Relation("info")

Could use some feedback on the API for this as well.

[marwan-at-work]

My opinion is still that the best API is no API -- in other words, if the foreign keys came out of the box as fields without any extra method, that's the most ideal. @a8m is the expert here so I'll definitely go with the flow 👍🏼

[adayNU]

I think I tend to agree, though it may be nice to be able to optionally explicitly set the name, add struct tags, etc.

[adayNU]

Thought about my idea a bit more over the weekend, and realized it probably makes a bunch more sense to define the FK Field name (and tags) on the edge directly vs as a field where you then need to pass the relation. Here's a stab at this idea: master...adayNU:idea/fks-one-edge-def

I think this is a lot cleaner. LMK your thoughts @a8m

edge.To("info", GroupInfo.Type).
	Unique().
	Required().
	FKFieldName("group_info_id").
	FKTag(`json:"group_info_id,omitempty"`)

[a8m]

Hey all, sorry for the delay. I was a little bit busy this week.
I'm going to comment here tomorrow morning my thoughts and hopefully come up with a final solution this week. I appreciate all comments and thanks for the feedback 🙏

[a8m]

I still think it should be gated with feature-flag at the moment, and we can move it to the default codegen when we feel it's in a good state. Regarding the API, I agree with @marwan-at-work and think we can change the implementation to auto-load the FKs and expose them as public struct fields as follows:

p := client.Pet.Query().FirstX(ctx)
p.OwnerID // (*int)

The type of the FK field will be pointer to int/string/uuid/etc if the edge is not required, and may hold the zero value (e.g. 0, ""), if the database column contains NULL.

Thoughts?

[adayNU]

That sounds great to me. Any thoughts on my proposal to be able to explicitly name / tag the FK ID field (happy to do that separate from this change)?

[aight8]

Just as input as another implementation variant.

Experimental feature could enable an extra struct field on the node:

node.EdgeIDs.MyRelation
// not MyRelationID because it's already under EdgeIDs

Similar than:

node.Edges.MyRelation

However my own opinion is, to skip the Edges sub struct at all and just let me use

node.MyRelation
node.MyRelationID

// currently i have implemented via template a edge-lazy function:
node.MyRelation()

[a8m]

Hey all, another update regarding the API. I was thinking on all other suggestions here again (and consulted with Marwan), and I would like to suggest another way to expose the foreign-keys as fields.

We'll add another field-builder to package schema/field called field.ID(field, edge string) (maybe field.EdgeID or field.Edge instead of field.ID?) that will help exposing the edge-field (i.e. foreign-key in SQL) on the generated struct. It will also generate for it predicates in the type-package.

Why not call named it "ForeignKey"? Because the plan is to support this option in other storage-drivers in the future (e.g. Gremlin).

The definition is as follows:

func (Pet) Fields() []ent.Field {
	return []ent.Field{
		field.String("name"),
		field.EdgeID("owner_id", "owner").
			StructTag("..."),
	}
}

func (Pet) Edges() []ent.Edge {
	return []ent.Edge{
		edge.From("owner", User.Type).
			Ref("pets"),
	}
}

Thoughts?

[adayNU]

I like it quite a bit, not too far off from my first suggestion :)

[rubensayshi]

I prefer the always-on adding of the FK when there's an edge, but I wouldn't hate your suggestion either ;)

[a8m]

Hey all (again 😄),
After implementing this feature 4 times differently, I finished it and decided to go with the following API:

The change to the ent.Schema is only one method we add to the ent.Edge builder named Field, and the ent.Field stays the same without any changes.

When users want to bind an edge to a foreign-key (called edge-field internally), they define it in the schema like any other fields, and point the edge to it using the method mentioned above:

// Fields of the Post.
func (Post) Fields() []ent.Field {
	return []ent.Field{
		field.Int("author_id").
			Optional(),
	}
}

// Edges of the Post.
func (Post) Edges() []ent.Edge {
	return []ent.Edge{
		edge.To("author", User.Type).
			Field("author_id").
			Unique(),
	}
}

When the framework sees an edge-field, it automatically generates predicates to it (=, <>, IN, NOT IN), and loads it (on queries) with all other fields by default.

p := client.Post.Query().Where(post.AuthorID(a8m.ID)).OnlyX(ctx)
fmt.Println(p.AuthorID) // 1

I'm going to land it later today, add documentation to it (along with some caveats for edge-migrations), and start working on the migration framework.

Thanks for all feedback here, it's really appreciated 🙏

[marwan-at-work]

@a8m I think that's a very clear design 👍🏼

Looking forward to using it!