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

Allow Union types as model fields #25

Closed

Conversation

CarpeNecopinum
Copy link

It would be interesting to allow the use of models like:

@kwdef mutable struct Book <: AbstractModel
    ### INTERNALS
    _table_name::String = "books"
    _id::String = "id"

    ### FIELDS
    id::DbId = DbId()
    title::String = ""
    author::String = ""
    cover::Union{String,Missing} = missing
end

Where the cover is effectively optional and by default missing. Unfortunately,

setfield!(obj, unq_field, oftype(getfield(_m, unq_field), value))

blocks this from working, as oftype(Book.cover, x) will call convert(Missing, x) which doesn't work for x being e.g. a String.

This PR changes that behavior in that we convert to the type of the field, rather than the type of its default value. The added function _union_convert handles cases like converting a Float64 to Union{Int,Nothing}, which would not work with Base.convert.

@essenciary
Copy link
Member

@CarpeNecopinum Sorry, OMG, I missed this for so long!
Is this necessary? Doesn't Julia handle the types if we just define the union, supporting all the types in the union?

@CarpeNecopinum
Copy link
Author

I haven't used Genie for like half a year now, so I can't say how it is now for sure. But at the time of writing, SearchLight would always try to convert values read from the database into the concrete type (in the above example Missing) rather than the field type (Union{String,Missing}).

The _union_convert then is a second step that ensures better backwards compatibility, because you can't convert e.g. a Float64 into a Union{Int,Missing}, but you can convert it to Int. But that second part is basically optional, wheras the first part (https://github.com/GenieFramework/SearchLight.jl/pull/25/files#diff-15442cb6a676563c8cd8d8903211bbd55fadcdfeb0eca3f6bcd493cef2ac1d4cL700) is needed to support Union fields.

@UniqueTokens
Copy link

+1
Union types as model fields would be particularly useful for is_nullable columns in PostgreSQL.
Any news on this?

@essenciary
Copy link
Member

Related to current discussions here: #55
Closing this issue in favour of the newer one.

@essenciary essenciary closed this May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants