vapor / fluent

Vapor ORM (queries, models, and relations) for NoSQL and SQL databases
https://docs.vapor.codes/4.0/fluent/overview/
MIT License
1.3k stars 171 forks source link

OptionalParent is not so optional #757

Closed Samsv77 closed 1 year ago

Samsv77 commented 1 year ago

Describe the bug

When tagging a field as OptionalParent, Fluent considers that the column can be set to null, however if the column is not set to null, and the "joined" table doesn't have a matching record, it throws the error "Parent missing".

I have 2 tables with no foreign key enforcement. OptionalParent should be fully optional, if the column is null, or if there is no record in the "joined" table, it should not fail. This is particularly true because Fluent doesn't join the tables when using .with(), it creates a separate SELECT query. So there should be no reason to enforce a foreign key in this case.

Expected behavior

OptionalParent should be fully optional, if the column is null, or if there is no record in the "joined" table, it should not fail.

Environment

0xTim commented 1 year ago

I think this is expected behaviour. Fluent makes no guarantees about how eager loading works (we would like to use joins where possible in the future for example) and we can change the implementations under the hood. There are a lot of assumptions in Fluent about constraints as well so using an @OptionalParent with a ID that doesn't exist (instead of it being nil - that's your issue correct?) would break some of Fluent's internals.

You're probably best off using an @OptionalField instead to hold the ID and doing the lookups yourself, including handling if that ID is not valid

gwynne commented 1 year ago

It sounds essentially as if you're hoping to get the effect of a left outer join, while the Fluent relation properties are designed to provide the effect of an inner join.

If you want this behavior, you can copy the contents of OptionalParent.swift into a new file in your project, rename the wrapper (maybe something like @UncheckedOptionalParent), and replace the logic that throws an error with your preferred outcome. This being said, Fluent does not officially support third-party packages providing custom property wrappers, so I can't promise that doing this will continue to work on an ongoing basis. (Although, I do use several custom wrappers in external projects myself, so odds are I won't break the ability to do so too badly 😅.)