Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Association between SQL and memory relations #323

Open
cutalion opened this issue Mar 6, 2018 · 6 comments
Open

Association between SQL and memory relations #323

cutalion opened this issue Mar 6, 2018 · 6 comments
Assignees
Labels

Comments

@cutalion
Copy link

cutalion commented Mar 6, 2018

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

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
Copy link
Member

solnic commented Mar 6, 2018

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 cutalion changed the title Issue with sql-to-memory association/combine. Assiciation between SQL and memory relations Mar 9, 2018
@cutalion cutalion changed the title Assiciation between SQL and memory relations Association between SQL and memory relations Mar 9, 2018
@cutalion
Copy link
Author

cutalion commented Mar 9, 2018

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
Copy link
Member

@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
Copy link
Member

solnic commented Mar 11, 2018

@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
Copy link
Collaborator

ianks commented Oct 15, 2018

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
Copy link
Member

solnic commented Oct 15, 2018

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

@solnic solnic self-assigned this Mar 27, 2019
@solnic solnic transferred this issue from rom-rb/rom Apr 18, 2019
@solnic solnic added the bug label Apr 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants