rom-sql icon indicating copy to clipboard operation
rom-sql copied to clipboard

Support for many_through_many associations

Open dikond opened this issue 8 years ago • 10 comments

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 avatar Aug 18 '17 08:08 dikond

@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

dikond avatar Aug 22 '17 11:08 dikond

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

solnic avatar Aug 22 '17 11:08 solnic

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

solnic avatar Aug 23 '17 11:08 solnic

Please take a look at what I did in #238

solnic avatar Aug 23 '17 11:08 solnic

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

dikond avatar Aug 23 '17 21:08 dikond

@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.

solnic avatar Aug 24 '17 08:08 solnic

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

dikond avatar Aug 24 '17 20:08 dikond

have FKs*

dikond avatar Aug 24 '17 20:08 dikond

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

solnic avatar Aug 24 '17 23:08 solnic

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.

solnic avatar Sep 11 '17 09:09 solnic