rom-rb / rom-sql

SQL support for rom-rb
https://rom-rb.org
MIT License
217 stars 93 forks source link

Association between SQL and memory relations #323

Open cutalion opened 6 years ago

cutalion commented 6 years ago

Association from sql relation to memory relation does not seem to work.

  config.relation(:companies) do # default adapter is sql
    schema(:companies, infer: true) do
      associations do
        belongs_to :market_data, foreign_key: :symbol, view: :capitalization, override: true
      end
    end
  end

  config.relation(:market_data, adapter: :memory) do
    # do we need schema?
    schema do
      attribute :symbol, ROM::Types::String
      attribute :cap, ROM::Types::String
    end

    def capitalization(_assoc, companies)
      MARKET.select do |company|
        companies.pluck(:symbol).include? company[:symbol]
      end
    end
  end
Full example

```ruby require 'rom' require 'rom-repository' require 'rom-sql' require 'logger' require 'pry' gateways = { default: [:sql, 'sqlite::memory'], memory: [:memory] } rom = ROM.container(gateways) do |config| sql = config.gateways[:default].connection sql.create_table(:companies) do primary_key :id column :name, String column :symbol, String end config.relation(:companies) do schema(:companies, infer: true) do associations do belongs_to :market_data, foreign_key: :symbol, view: :capitalization, override: true end end end config.relation(:market_data, adapter: :memory) do # do we need schema? schema do attribute :symbol, ROM::Types::String attribute :cap, ROM::Types::String end def capitalization(_assoc, companies) MARKET.select do |company| companies.pluck(:symbol).include? company[:symbol] end end end end companies = rom.relations[:companies] companies.changeset(:create, [{ name: 'Google', symbol: 'GOOG' }]).commit companies.changeset(:create, [{ name: 'Facebook', symbol: 'FB' }]).commit companies.changeset(:create, [{ name: 'Apple', symbol: 'AAPL' }]).commit # Our external source of market data MARKET = [ { symbol: 'FB', cap: '518B' }, { symbol: 'GOOG', cap: '754B' }, { symbol: 'AAPL', cap: '895B' } ] result = companies .where(symbol: ['FB', 'AAPL']) .combine(:market_data) ```

$ ruby sql_memory_combine.rb
/home/cutalion/.rvm/gems/ruby-2.4.1/gems/rom-sql-2.4.0/lib/rom/sql/associations/many_to_one.rb:15:in `call': undefined method `qualified' for #<ROM::Memory::Schema:0x00000003971910> (NoMethodError)
    from /home/cutalion/.rvm/gems/ruby-2.4.1/gems/rom-sql-2.4.0/lib/rom/sql/associations/many_to_one.rb:50:in `prepare'
    from /home/cutalion/.rvm/gems/ruby-2.4.1/gems/rom-core-4.1.2/lib/rom/relation.rb:301:in `eager_load'
    from /home/cutalion/.rvm/gems/ruby-2.4.1/gems/rom-core-4.1.2/lib/rom/relation.rb:290:in `node'
    from /home/cutalion/.rvm/gems/ruby-2.4.1/gems/rom-core-4.1.2/lib/rom/support/memoizable.rb:47:in `block (2 levels) in define_memoizable_names!'
    from /home/cutalion/.rvm/gems/ruby-2.4.1/gems/rom-core-4.1.2/lib/rom/relation.rb:271:in `block in nodes'
    from /home/cutalion/.rvm/gems/ruby-2.4.1/gems/rom-core-4.1.2/lib/rom/relation.rb:268:in `each'
    from /home/cutalion/.rvm/gems/ruby-2.4.1/gems/rom-core-4.1.2/lib/rom/relation.rb:268:in `reduce'
    from /home/cutalion/.rvm/gems/ruby-2.4.1/gems/rom-core-4.1.2/lib/rom/relation.rb:268:in `nodes'
    from /home/cutalion/.rvm/gems/ruby-2.4.1/gems/rom-core-4.1.2/lib/rom/relation.rb:252:in `combine'
    from /home/cutalion/.rvm/gems/ruby-2.4.1/gems/rom-core-4.1.2/lib/rom/support/memoizable.rb:47:in `block (2 levels) in define_memoizable_names!'
    from sql_memory_combine.rb:60:in `<main>'

Download example

solnic commented 6 years ago

This is clearly a bug. rom-sql assumes that the other schema must be qualified too. I'm not entirely sure how to solve it at the moment, but probably adding this interface to core wouldn't be such a bad idea. WDYT @flash-gordon ?

cutalion commented 6 years ago

I'm not sure it would make sense for other gateways/adapters (how do call all rom-whatever gems?) What if we qualify schema right before calling it. In the ROM::SQL::Schema#call method?

flash-gordon commented 6 years ago

@solnic but we qualified schemas in rom-sql 2.0, I believe those .qualified calls aren't even needed. Specs from rom-sql and rom/repo passing

solnic commented 6 years ago

@flash-gordon I thought about it before 2.0 final was released, but I realized it would create an implicit requirement that associations can only work when relations are qualified, and you may end up with un-qualified attributes by a mistake...on the other hand, current auto-qualification in associations is...implicit behavior 😆 So, we could try removing it and test it out with real apps to make sure (at least we) don't have use cases where some code actually relies on auto-qualification in associations.

ianks commented 5 years ago

We ran into this issue as well. Kind of a bummer and now we are just going to move the yaml relation to sql

solnic commented 5 years ago

OK let's finally address it in 5.0 and rom-sql 3.0