Open CarpeNecopinum opened 4 years ago
Oof, forgot to change
Book(;
id = DbId(),
title = "",
author = "",
cover = ""
) = new("books", "id", id, title, author)
to include the cover in the new
call. In combination with SearchLight skipping missing
s when turning a row into a model instance, that caused the issue above.
Pretty hard to find this based on the error message alone, though. Would be cool if some way of reducing potential for this error could be implemented. (Maybe suggest using @kwdef
in the tutorial?)
@CarpeNecopinum Thanks, happy to take a look as I'm revamping SearchLight, but can you instruct how to reproduce?
Ah, I understand, one of the fields is not added to the model, but it's added to the table... Interesting, I'll test.
I was missing the cover
in the Book
constructor, so it was left as an undefined reference. SearchLight then sees the NULL
in the database (or rather the missing
it gets from SQLite.jl) and decides to not touch the cover
field of Book
. Later then, that causes issues when the Book is supposed to be printed.
I tested @kwargs
by now and it appears to work with Genie. So maybe we should just change the tutorial to suggest
using Base: @kwdef
@kwdef mutable struct Book <: AbstractModel
### INTERNALS
_table_name::String = "books"
_id::String = "id"
### FIELDS
id::DbId = DbId()
title::String = ""
author::String = ""
cover::String = ""
end
That way one's less likely to write a broken constructor for your models.
Sounds great!
The "internal" fields will be gone in a few days as I'm refactoring to the idiomatic Julia way. So we'll just specialise methods as in
# in Books.jl
function SearchLight.table_name(b::Book)
And probably have a default
# in SearchLight.jl
function table_name(m::T) where {T<:AbstractModel}
# logic for default table name, ie return "books" from `Book`
end
This approach should considerably simplify the API and cleanup the code.
Yes, @kwdef
is fantastic! Coupled with the removal of "internals" it will be a huge improvement. Love it!
@CarpeNecopinum Some early experiments, this looks great!
module Cars
using SearchLight
import Base: @kwdef
export Car
@kwdef mutable struct Car <: AbstractModel
id::DbId = DbId()
make::String = ""
max_speed::Int = 220
end
SearchLight.table_name()::String = "cars" # name of the DB table
SearchLight.pk()::String = "id" # name of the table's primary key
end
Shouldn't it be more like SearchLight.table_name(::Type{Car}) = "cars"
there (similarly for pk()
)?
... otherwise you'd only always get the name of the last model class you loaded.
But yeah, I really like the idea of moving the table name and key column away from the model instances. Makes things much cleaner.
Yes, my bad, it takes the model as the argument! 😁
Playing through the Genie "Bill Gates Books" tutorial. The migration to the second version of the DB (i.e. adding the
cover
column) breaks SearchLight:Appears whenever
all(Book)
is ran.This appears to be due to handling of missing values. Opening the database in SQLite.jl the
cover
column in particular is typed asUnion{Missing,String}
. So I assume the problem springs from trying to turn aMissing
into aString
as required by theBook
model struct.Changing the type of
cover
toUnion{Missing,String}
leads to the same error.