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

has_many through join generates incorrect query #279

Closed
blelump opened this issue Mar 21, 2018 · 1 comment · Fixed by #328
Closed

has_many through join generates incorrect query #279

blelump opened this issue Mar 21, 2018 · 1 comment · Fixed by #328

Comments

@blelump
Copy link

blelump commented Mar 21, 2018

Hi!

I'd like to report a problem with simple has_many through usage. Lets consider simple users has many employees case with intersection table employees_users. For a join on employees, ROM generates query:

SELECT `users`.`id`, `users`.`login` FROM `users` 
INNER JOIN `employees_users` ON (`users`.`id` = `employees_users`.`user_id`) 
INNER JOIN `employees` ON (`users`.`id` = `employees_users`.`user_id`) 
WHERE (`users`.`id` = 1) ORDER BY `users`.`id`

whereas it should be (compare join on employees):

SELECT `users`.`id`, `users`.`login` FROM `users` 
INNER JOIN `employees_users` ON (`users`.`id` = `employees_users`.`user_id`) 
INNER JOIN `employees` ON (`employees`.`id` = `employees_users`.`employee_id`) 
WHERE (`users`.`id` = 1) ORDER BY `users`.`id`

It helps a bit, when you'll provide foreign_key on relation associations, ie:

  conf.relation(:users) do
    schema(:users, infer: true) do
      associations do
        has_many :employees_users, foreign_key: :user_id
        has_many :employees, through: :employees_users, foreign_key: :employee_id
      end
    end
  end

but still, the join on employees in incorrect.

See the code below to recreate the problem.

rom = ROM.container(:sql, 'sqlite::memory') do |conf|
  conf.gateways[:default].use_logger(Logger.new($stdout))

  conf.default.create_table(:users) do
    primary_key :id
    column :login, String, null: false
  end

  conf.default.create_table(:employees) do
    primary_key :id
    column :name, String, null: false
  end

  conf.default.create_table(:employees_users) do
    foreign_key :employee_id, :employees
    foreign_key :user_id, :users
  end

  conf.relation(:users) do
    schema(:users, infer: true) do
      associations do
        has_many :employees_users
        has_many :employees, through: :employees_users
      end
    end
  end

  conf.relation(:employees) do
    schema(:employees, infer: true) do
      associations do

      end
    end
  end

  conf.relation(:employees_users) do
    schema(:employees_users, infer: true) do
      associations do
        belongs_to :user
        belongs_to :employee
      end
    end
  end
end

class UserRepo < ROM::Repository[:users]
  def by_id(id)
    users.join(:employees)
      .by_pk(id)
      .one
  end
end

employee = rom.relations[:employees].changeset(:create, name: 'Jane').commit

user = rom.relations[:users]
  .changeset(:create, login: '[email protected]')
  .commit

rom.relations[:employees_users]
  .changeset(:create, user_id: user[:id], verifable_id: employee[:id]).commit

repo = UserRepo.new(rom)

user = repo.by_id(1)

Bundled with:
rom-sql (2.3.0)

@blelump blelump changed the title has_many through generates incorrect query has_many through join generates incorrect query Mar 21, 2018
@doriantaylor
Copy link
Contributor

looks like i inadvertently fixed this; patch forthcoming

doriantaylor added a commit to doriantaylor/rom-sql that referenced this issue May 22, 2018
doriantaylor added a commit to doriantaylor/rom-sql that referenced this issue May 22, 2018
solnic pushed a commit that referenced this issue Apr 20, 2019
solnic pushed a commit that referenced this issue Apr 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants