rom-rb / rom-sql

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

Support for many_through_many associations #236

Open dikond opened 7 years ago

dikond commented 7 years ago

Hi! This issue was extracted from the gitter discussion, here is the link.

The following example will tell better than I will.

class Eans < ROM::Relation[:sql]
  schema(:eans, infer: true) do
    associations do
      has_many :ean_stats
      has_many :contract_ean_stats, through: :ean_stats
      has_many :contracts,          through: :contract_ean_stats
      has_many :companies,          through: :contracts
    end
  end
end

eans.combine(:contract_ean_stats).limit(1).one
# => ROM::Struct::Ean
eans.combine(:contracts).limit(1).one
# => KeyError: :ean_id attribute doesn't exist in contract_ean_stats schema
dikond commented 7 years ago

@solnic here is a self-contained snippet, just in case.

require 'rom'
require 'sqlite3'

rom = ROM.container(:sql, 'sqlite::memory') do |conf|
  conf.default.create_table(:eans) do
    primary_key :id
  end

  conf.default.create_table(:ean_stats) do
    primary_key :id
    column :ean_id, Integer
  end

  conf.default.create_table(:contract_ean_stats) do
    primary_key :id
    column :contract_id, Integer
    column :ean_stat_id, Integer
  end

  conf.default.create_table(:contracts) do
    primary_key :id
  end

  conf.relation(:eans) do
    schema(infer: true) do
      associations do
        has_many :ean_stats
        has_many :contract_ean_stats, through: :ean_stats
        has_many :contracts,          through: :contract_ean_stats
      end
    end
  end

  conf.relation(:ean_stats) do
    schema(infer: true) do
      associations do
        belongs_to :ean
        has_many :contract_ean_stats
      end
    end
  end

  conf.relation(:contract_ean_stats) do
    schema(infer: true) do
      associations do
        belongs_to :contract
        belongs_to :ean_stat
      end
    end
  end

  conf.relation(:contracts) do
    schema(infer: true) do
      associations do
        has_many :contract_ean_stats
        has_many :ean_stats, through: :contract_ean_stats
      end
    end
  end
end

rom.relations[:eans].combine(:contract_ean_stats).one
# => nil
rom.relations[:eans].combine(:contracts).one
# => KeyError: :ean_id attribute doesn't exist in contract_ean_stats schema
solnic commented 7 years ago

@dikond thanks, this is very helpful. I'll address this tomorrow.

solnic commented 7 years ago

@dikond is this a legacy schema? any reason why there are no real foreign keys in the database?

solnic commented 7 years ago

Please take a look at what I did in #238

dikond commented 7 years ago

Hey @solnic, the schema isn't legacy. When we designed it we haven't thought about this association. But later on we had to associate eans with companies and it turned out we can do it with many_through_many (in Sequel). Maybe it isn't the best way but it just works and we use this association not that often.

If your spec is passing then I believe my problem is solved (although I cannot try it myself until the 1-2 of September). However, I am not sure if it really closes this issue. BTW, I saw your tweet and now I think there is something horrible going on with these type of assocs, but I just do not imagine :D

solnic commented 7 years ago

@dikond the problem is that there are no FKs defined (at least in the example you attached), and rom-sql doesn't infer FKs from a schema when...there are no FKs :) The fact that a column is called "foo_id" isn't enough for rom-sql, it has to be a real FK.

dikond commented 7 years ago

Oh, that actually may be mine mistake, because all columns with '_id' suffix are FKs

dikond commented 7 years ago

have FKs*

solnic commented 7 years ago

@dikond OK, so I'll tweak this setup to use real FKs and look into this again :)

solnic commented 7 years ago

ok, I spent another 1.5h on this association scenario. Eventually I realized this requires a completely separate association type, which is a new feature. What we can quickly do now, is raise a meaningful error when foreign key is not found via join relation. We can add support for this type of associations in rom-sql 2.1.