elixir-sqlite/ecto_sqlite3

Ecto expectations for map fields

Closed this issue ยท 7 comments

Howdy,

I'm trying to use the polymorphic_embed library with the SQLite3 ecto adapter and I'm running into an error loading the map field. It works correctly with the ecto postgres adapter. It seems like ecto is expecting JSON fields to be deserialized from a string to a struct.

1) test returns the oldest 25 order transitions by default (Tai.Orders.SearchTransitionsTest)                                                                                                      
     apps/tai/test/tai/orders/search_transitions_test.exs:4                                                                                                                                          
     ** (FunctionClauseError) no function clause matching in PolymorphicEmbed.do_get_polymorphic_module_from_map/3                                                                                   
                                                                                                                                                                                                     
     The following arguments were given to PolymorphicEmbed.do_get_polymorphic_module_from_map/3:                                                                                                    
                                                                                                                                                                                                     
         # 1                                                                                                                                                                                         
         "{\"last_received_at\":\"2021-07-23T06:42:10.671042Z\",\"last_venue_timestamp\":\"2021-07-23T06:42:10.671036Z\",\"__type__\":\"cancel\"}"                                                   
                                                                                                                                                                                                     
         # 2                                                                                                                                                                                         
         "__type__"                                                                                                                                                                                  
                                                                                                                                                                                                     
         # 3                                                                                                                                                                                         
         [%{identify_by_fields: [], module: Tai.Orders.Transitions.AcceptCreate, type: "accept_create"}, %{identify_by_fields: [], module: Tai.Orders.Transitions.VenueCreateError, type: "venue_crea
te_error"}, %{identify_by_fields: [], module: Tai.Orders.Transitions.RescueCreateError, type: "rescue_create_error"}, %{identify_by_fields: [], module: Tai.Orders.Transitions.Open, type: "open"}, %
{identify_by_fields: [], module: Tai.Orders.Transitions.PendCancel, type: "pend_cancel"}, %{identify_by_fields: [], module: Tai.Orders.Transitions.AcceptCancel, type: "accept_cancel"}, %{identify_b
y_fields: [], module: Tai.Orders.Transitions.VenueCancelError, type: "venue_cancel_error"}, %{identify_by_fields: [], module: Tai.Orders.Transitions.RescueCancelError, type: "rescue_cancel_error"},
 %{identify_by_fields: [], module: Tai.Orders.Transitions.Cancel, type: "cancel"}, %{identify_by_fields: [], module: Tai.Orders.Transitions.PendAmend, type: "pend_amend"}, %{identify_by_fields: [],
 module: Tai.Orders.Transitions.AcceptAmend, type: "accept_amend"}, %{identify_by_fields: [], module: Tai.Orders.Transitions.VenueAmendError, type: "venue_amend_error"}, %{identify_by_fields: [], m
odule: Tai.Orders.Transitions.RescueAmendError, type: "rescue_amend_error"}, %{identify_by_fields: [], module: Tai.Orders.Transitions.Amend, type: "amend"}, %{identify_by_fields: [], module: Tai.Or
ders.Transitions.Fill, type: "fill"}, %{identify_by_fields: [], module: Tai.Orders.Transitions.PartialFill, type: "partial_fill"}, %{identify_by_fields: [], module: Tai.Orders.Transitions.Expire, t
ype: "expire"}, %{identify_by_fields: [], module: Tai.Orders.Transitions.Reject, type: "reject"}, %{identify_by_fields: [], module: Tai.Orders.Transitions.Skip, type: "skip"}]                      
                                                                                                                                                                                                     
     Attempted function clauses (showing 1 out of 1):                                                                                                                                                
                                                                                                                                                                                                     
         defp do_get_polymorphic_module_from_map(%{} = attrs, type_field, types_metadata)                                                                                                            
                                                                                                                                                                                                     
     code: search_order_transitions = Tai.Orders.search_transitions(order.client_id, nil)                                                                                                            
     stacktrace:                                                                                                                                                                                     
       (polymorphic_embed 1.6.3) lib/polymorphic_embed.ex:286: PolymorphicEmbed.do_get_polymorphic_module_from_map/3                                                                                 
       (polymorphic_embed 1.6.3) lib/polymorphic_embed.ex:248: PolymorphicEmbed.load/3                                                                                                               
       (ecto 3.6.2) lib/ecto/type.ex:894: Ecto.Type.process_loaders/3                                                                                                                                
       (ecto 3.6.2) lib/ecto/repo/queryable.ex:406: Ecto.Repo.Queryable.struct_load!/6                                                                                                               
       (ecto 3.6.2) lib/ecto/repo/queryable.ex:238: anonymous fn/5 in Ecto.Repo.Queryable.preprocessor/3                                                                                             
       (elixir 1.11.4) lib/enum.ex:1411: Enum."-map/2-lists^map/1-0-"/2
       (ecto 3.6.2) lib/ecto/repo/queryable.ex:229: Ecto.Repo.Queryable.execute/4
       (ecto 3.6.2) lib/ecto/repo/queryable.ex:19: Ecto.Repo.Queryable.all/3
       test/tai/orders/search_transitions_test.exs:8: (test)

I've created a branch to reproduce to problem

https://github.com/fremantle-industries/tai/tree/sqlite3-polymorphic-embed-invalid-map-field

$ mix test test/tai/orders/search_transitions_test.exs

The embed field is defined in the OrderTransition schema

https://github.com/fremantle-industries/tai/blob/sqlite3-polymorphic-embed-invalid-map-field/apps/tai/lib/tai/orders/order_transition.ex#L20

I'm fairly certain this is due to differences between postgrex and exqlite handle json.

exqlite is pretty low level and only handles the 5 actual SQLite data types. This means that we do the mapping from the JSON string into a map in the Ecto adapter layer (i.e., this library).

postgrex, by comparison, handles the full set of Postgres data types (which includes e.g., jsonb) and does the conversion into the Elixir equivalents in the driver before it reaches the Ecto layer. I think the myxql adapter does hte same.

My idea is that is the cause of the issue. This library is not letting the data returned from the query go through the full Ecto "pipeline" and allowing our loaders to be run. Specifically these lines at lib/ecto/adapters/sqlite3.ex (note that we do the JSON decode here):

  @impl Ecto.Adapter
  def loaders({:map, _}, type) do
    [&Codec.json_decode/1, &Ecto.Type.embedded_load(type, &1, :json)]
  end

  @impl Ecto.Adapter
  def loaders({:array, _}, type) do
    [&Codec.json_decode/1, type]
  end

  @impl Ecto.Adapter
  def loaders(:map, type) do
    [&Codec.json_decode/1, type]
  end

The fix is either:

  1. Update the polymorphic_embed library to run the associated loaders first before it does its thing
  2. Update the polymorphic_embed library to decode the JSON string on its own if it detects it gets a string and not a map before it does its thing
  3. Update exqlite to have more conversion scope

Either (1) or (2) I think is the preferred solution.exqlite tries to stay as true to SQLite3 actual interface as possible - and SQLite3 simply does not return anything other than a string representation of the json column, as it only has 5 actual data types.

I hope that makes sense. You may need to coordinate with the maintainer of that library for the fix. I'll keep this issue open for now, though I don't think there is anything on our end we can do.

--

Edit: If (1) or (2) above end up being ill-advised, clunky, or not possible when consulting with the maintainer of polymorphic_embed we can further explore (3).

I'm curious if this is also an issue with the MySQL adapter. I based a lot of the sqlite3 adapter off of it.

@mathieuprog this is the same issue I posted in the polymorphic_embed repo mathieuprog/polymorphic_embed#39. When you get a chance do you mind reporting back how we should move forward with it?

Thanks for the detailed response @kevinlang

If it is a limitation (purposely) of exqlite, then we can convert the json string into a map. I don't see how solution (1) works.

Yep, I think (1) would not work. I'm not too familiar with ParamaterizedTypes, but reading through briefly I now understand that ultimately your library is its own "type", so we wouldn't have any loader to process it anyway.

I agree that (2) seems the way to go, if we do not want to make changes to exqlite (I defer to @warmwaffles on that one).


@warmwaffles I don't think it is an issue with the MySQL driver, based on this file:
https://github.com/elixir-ecto/myxql/blob/fdb147dba07d7699c4af23448996ae7f4663bce6/lib/myxql/protocol/values.ex

There I see that the driver is encoding the json column response directly into an Elixir map, so the ParamaterizedType would get that value to process on load.

We probably need to make if function similarly.

Closing as @mathieuprog has merged a fix. Cheers @warmwaffles @kevinlang.