NULL in database triggers UndefRefError

[CarpeNecopinum]

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:

┌ Error: 2020-02-10 16:28:59 UndefRefError()
└ @ SearchLight .../packages/SearchLight/syIVM/src/SearchLight.jl:716
┌ Error: 2020-02-10 16:28:59 cover
└ @ SearchLight .../packages/SearchLight/syIVM/src/SearchLight.jl:717
┌ Error: 2020-02-10 16:28:59 UndefRefError()
└ @ SearchLight .../packages/SearchLight/syIVM/src/SearchLight.jl:716
┌ Error: 2020-02-10 16:28:59 cover
└ @ SearchLight .../packages/SearchLight/syIVM/src/SearchLight.jl:717
┌ Error: 2020-02-10 16:28:59 UndefRefError()
└ @ SearchLight .../packages/SearchLight/syIVM/src/SearchLight.jl:716
┌ Error: 2020-02-10 16:28:59 cover
└ @ SearchLight .../packages/SearchLight/syIVM/src/SearchLight.jl:717
┌ Error: 2020-02-10 16:28:59 UndefRefError()
└ @ SearchLight .../packages/SearchLight/syIVM/src/SearchLight.jl:716
┌ Error: 2020-02-10 16:28:59 cover
└ @ SearchLight .../packages/SearchLight/syIVM/src/SearchLight.jl:717
┌ Error: 2020-02-10 16:28:59 UndefRefError()
└ @ SearchLight .../packages/SearchLight/syIVM/src/SearchLight.jl:716
┌ Error: 2020-02-10 16:28:59 cover
└ @ SearchLight .../packages/SearchLight/syIVM/src/SearchLight.jl:717
┌ Error: 2020-02-10 16:28:59 UndefRefError()
└ @ SearchLight .../packages/SearchLight/syIVM/src/SearchLight.jl:716
┌ Error: 2020-02-10 16:28:59 cover
└ @ SearchLight .../packages/SearchLight/syIVM/src/SearchLight.jl:717

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 as Union{Missing,String}. So I assume the problem springs from trying to turn a Missing into a String as required by the Book model struct.

Changing the type of cover to Union{Missing,String} leads to the same error.

pkg> status SearchLight
    Status `.../CRUDtest/Project.toml`
  [295af30f] Revise v2.5.0
  [0aa819cd] SQLite v0.9.0
  [340e8cb6] SearchLight v0.17.0 #master

julia> versioninfo()
Julia Version 1.3.1
Commit 2d5741174c (2019-12-30 21:36 UTC)
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: Intel(R) Core(TM) i7-7700K CPU @ 4.20GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-6.0.1 (ORCJIT, skylake)
Environment:
  JULIA_DEPOT_PATH = .../julia-depot
  JULIA_NUM_THREADS = 8
  JULIA_EDITOR = atom  -a
  JULIA_REVISE = auto

[CarpeNecopinum]

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 missings 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?)

[essenciary]

@CarpeNecopinum Thanks, happy to take a look as I'm revamping SearchLight, but can you instruct how to reproduce?

[essenciary]

Ah, I understand, one of the fields is not added to the model, but it's added to the table... Interesting, I'll test.

[CarpeNecopinum]

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.

[CarpeNecopinum]

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.

[essenciary]

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.

[essenciary]

Yes, @kwdef is fantastic! Coupled with the removal of "internals" it will be a huge improvement. Love it!

[essenciary]

@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

[CarpeNecopinum]

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.

[essenciary]

Yes, my bad, it takes the model as the argument! 😁