GenieFramework / SearchLight.jl

ORM layer for Genie.jl, the highly productive Julia web framework
https://genieframework.com
MIT License
139 stars 16 forks source link

SearchLight complains when an AbstractModel has a field with no corresponding db column. #45

Open mcvmcv opened 2 years ago

mcvmcv commented 2 years ago

Describe the bug Adding a field to an AbstractModel with no corresponding db column produces an error. I'm not sure if this is expected behaviour or not.

Error stacktrace

julia> all(Colour)
[ Info: 2022-01-18 08:42:01 SELECT "colours"."id" AS "colours_id", "colours"."name" AS "colours_name", "colours"."hex" AS "colours_hex", "colours"."other" AS "colours_other" FROM "colours" ORDER BY colours.id ASC
ERROR: SQLite.SQLiteException("no such column: colours.other")
Stacktrace:
  [1] sqliteerror(handle::Ptr{Nothing})
    @ SQLite ~/.julia/packages/SQLite/5DdLp/src/SQLite.jl:20
  [2] sqliteerror(db::SQLite.DB)
    @ SQLite ~/.julia/packages/SQLite/5DdLp/src/SQLite.jl:114
  [3] macro expansion
    @ ~/.julia/packages/SQLite/5DdLp/src/consts.jl:24 [inlined]
  [4] sqliteprepare
    @ ~/.julia/packages/SQLite/5DdLp/src/SQLite.jl:190 [inlined]
  [5] SQLite._Stmt(db::SQLite.DB, sql::String)
    @ SQLite ~/.julia/packages/SQLite/5DdLp/src/SQLite.jl:122
  [6] Stmt
    @ ~/.julia/packages/SQLite/5DdLp/src/SQLite.jl:152 [inlined]
  [7] prepare
    @ ~/.julia/packages/SQLite/5DdLp/src/tables.jl:104 [inlined]
  [8] execute(conn::SQLite.DB, sql::String, params::NamedTuple{(), Tuple{}})
    @ DBInterface ~/.julia/packages/DBInterface/1Gmxx/src/DBInterface.jl:130
  [9] #execute#2
    @ ~/.julia/packages/DBInterface/1Gmxx/src/DBInterface.jl:152 [inlined]
 [10] execute
    @ ~/.julia/packages/DBInterface/1Gmxx/src/DBInterface.jl:152 [inlined]
 [11] query(sql::String, conn::SQLite.DB)
    @ SearchLightSQLite ~/.julia/packages/SearchLightSQLite/cvPd7/src/SearchLightSQLite.jl:151
 [12] query
    @ ~/.julia/packages/SearchLightSQLite/cvPd7/src/SearchLightSQLite.jl:135 [inlined]
 [13] DataFrame
    @ ~/.julia/packages/SearchLight/B9d2o/src/SearchLight.jl:68 [inlined]
 [14] find(m::Type{Colour}, q::SearchLight.SQLQuery, j::Nothing) (repeats 2 times)
    @ SearchLight ~/.julia/packages/SearchLight/B9d2o/src/SearchLight.jl:85
 [15] #all#36
    @ ~/.julia/packages/SearchLight/B9d2o/src/SearchLight.jl:126 [inlined]
 [16] all(m::Type{Colour})
    @ SearchLight ~/.julia/packages/SearchLight/B9d2o/src/SearchLight.jl:126
 [17] top-level scope
    @ REPL[2]:1

To reproduce Create a resource "colour":

module Colours

import SearchLight: AbstractModel, DbId
using SearchLight.Validation, ColoursValidator
import Base: @kwdef

export Colour

@kwdef mutable struct Colour <: AbstractModel
    id::DbId = DbId()
    name::String = ""
    hex::String = ""
end

end

with a db migration:

module CreateTableColours

import SearchLight.Migrations: create_table, column, columns, primary_key, add_index, drop_table, add_indices

function up()
    create_table(:colours) do
        [
            primary_key()
            column(:name, :string)
            column(:hex, :string)
        ]
    end

    add_index(:colours, :name)
end

function down()
    drop_table(:colours)
end

end

This should work fine: you can create, save, update, etc. Colours.

Then edit Colours.jl:

@kwdef mutable struct Colour <: AbstractModel
    id::DbId = DbId()
    name::String = ""
    hex::String = ""
    other::String = ""
end

Then julia > all(Colour) or

julia > red = Colour(name = "red", hex = "FF0000", other = "foo")
Colour
| KEY                  | VALUE   |
|----------------------|---------|
| hex::String          | FF0000  |
| id::SearchLight.DbId | NULL    |
| name::String         | red     |
| other::String        | foo     |

julia > save!(red)

throw the above error.

Expected behavior The object "red" is saved to the db, ignoring the field "other".

Additional context Please include the output of julia> versioninfo() and pkg> st Additional context Please include the output of julia> versioninfo() and pkg> st

julia> versioninfo()
Julia Version 1.6.2
Commit 1b93d53fc4 (2021-07-14 15:36 UTC)
Platform Info:
  OS: Linux (x86_64-pc-linux-gnu)
  CPU: Intel(R) Core(TM) i3-6100 CPU @ 3.70GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-11.0.1 (ORCJIT, skylake)
Environment:
  JULIA_REVISE = auto

and

(Myproject) pkg> st
     Project Myproject v0.1.0
      Status `~/projects/Myproject/Project.toml`
  [336ed68f] CSV v0.9.11
  [c43c736e] Genie v4.9.0
  [e115e502] GenieAuthentication v1.1.0
  [6d011eab] Inflector v1.0.1
  [e6f89c97] LoggingExtras v0.4.7
  [739be429] MbedTLS v1.0.3
  [0aa819cd] SQLite v1.3.0
  [340e8cb6] SearchLight v2.0.1
  [21a827c4] SearchLightSQLite v2.0.0
  [4acbeb90] Stipple v0.19.3
  [30ddb3f0] StippleCharts v0.16.0
  [a3c5d34a] StippleUI v0.14.3
  [ade2ca70] Dates
  [56ddb016] Logging
mcvmcv commented 2 years ago

Some context from discord: 7/ It looks like there is confusion around whether you can specify a field in a subtype of AbstractModel that doesn't have a corresponding database column. I have found that SearchLight complains if I try to add one. Are there updates or something that I'm missing?

Essenciary: Normally it shouldn't - it just uses the fields that map column names. Can you open an issue with a MWE?

Franku: The thing you miss is the possibility to work with fields which are not stored in the database. I call them virtual fields in the environment of working with databases. They are not virtual because you can work with them inside your program but they will not be stored in the database. This could be reached by a technic I had implemented in the past which is gone with the PR. A specific function was defined to translate fields of the model into fields which are stored to the database. This works for two things. 1. You can use whatever field name you want in your model and provide a translation to the column name in the database. 2. You can specify the fields which have to be stored in the database and leave others which are only needed for working with the model, e.g You store the password in one field in its clear form and have a hashed field which will be stored in the database. I discussed this feature with Adrian and he was not disinclined to include such functionality into Searchlight.