rom-rb / rom-sql

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

relation.wrap should respect association view #251

Open solnic opened 6 years ago

solnic commented 6 years ago

From @johnnoone on June 9, 2017 10:49

I've defined 2 relations, one with a custom view (to filter some columns), and an association to that view.

When I use relation.combine_children, it works as expected and linked entity returns only the wanted fields. But when I use relation.wrap, it does not respect the custom view.

For example this script:

#!/usr/bin/env ruby
require 'rom-sql'
require 'rom-repository'
require 'dry-types'

module Types
  include Dry::Types.module
end

rom = ROM.container(:sql, 'sqlite::memory') do |c|
  c.gateways[:default].create_table :users do
    primary_key :id, Integer
    column :name, String
    column :secret, String
  end

  c.gateways[:default].create_table :goods do
    primary_key :id, Integer
    column :name, String
    column :owner_id, Integer
  end

  c.gateways[:default].use_logger(Logger.new($stdout))

  c.relation(:users) do
    schema(:users, infer: false) do
      attribute :id, Types::Int.meta(primary_key: true)
      attribute :name, Types::String
      attribute :secret, Types::String
    end

    view(:as_owner, [:id, :name]) do
      select(:id, :name)
    end
  end

  c.relation(:goods) do
    schema(:goods, infer: false) do
      attribute :id, Types::Int.meta(primary_key: true)
      attribute :name, Types::String
      attribute :owner_id, Types::Int.meta(foreign_key: true)

      associations do
        a = belongs_to :users, as: :owner, foreign_key: :owner_id, view: :as_owner
      end
    end
  end
end

jane = rom.relations[:users].insert(name: "Jane", secret: "not so secret")
fred = rom.relations[:users].insert(name: "Fred")

rom.relations[:goods].insert(name: 'pen', owner_id: jane)
rom.relations[:goods].insert(name: 'bag', owner_id: fred)

class GoodRepo < ROM::Repository[:goods]
  relations :users

  def query_with_combine
    goods
      .combine_children(one: { owner: users.as_owner })
      .to_a
  end

  def query_with_wrap
    goods
      .wrap(:owner)
      .to_a
  end
end

repo = GoodRepo.new(rom)

puts ""
puts "With combine, it returns wanted columns"
puts repo.query_with_combine.to_a.inspect

puts ""
puts "With wrap, it does not filters columns"
puts repo.query_with_wrap.to_a.inspect

Wrap query returns

SELECT `goods`.`id`,
       `goods`.`name`,
       `goods`.`owner_id`,
       `users`.`id` AS 'users_id',
       `users`.`name` AS 'users_name',
       `users`.`secret` AS 'users_secret'
FROM `goods`
  INNER JOIN `users` ON (`goods`.`owner_id` = `users`.`id`)
ORDER BY `goods`.`id`
[#<ROM::Struct::Good id=1 name="pen" owner_id=1 owner=#<ROM::Struct::Owner id=1 name="Jane" secret="not so secret">>,
 #<ROM::Struct::Good id=2 name="bag" owner_id=2 owner=#<ROM::Struct::Owner id=2 name="Fred" secret=nil>>]

Expected

SELECT `goods`.`id`,
       `goods`.`name`,
       `goods`.`owner_id`,
       `users`.`id` AS 'users_id',
       `users`.`name` AS 'users_name'
FROM `goods`
  INNER JOIN `users` ON (`goods`.`owner_id` = `users`.`id`)
ORDER BY `goods`.`id`
[#<ROM::Struct::Good id=1 name="pen" owner_id=1 owner=#<ROM::Struct::Owner id=1 name="Jane">>,
 #<ROM::Struct::Good id=2 name="bag" owner_id=2 owner=#<ROM::Struct::Owner id=2 name="Fred">>]

Copied from original issue: rom-rb/rom#407

solnic commented 6 years ago

I'm not sure if we want to support :view option along with wraps, because they use joins instead of composition so it's really hard to tell how such a custom view is supposed to be used. Good news is that in rom 4.0 wrap will be part of the core API (already in master) so that you'll be able to easily define your own relation view that uses wrap and do whatever you want there.

I'll leave this open for further discussion.

solnic commented 6 years ago

@flash-gordon do you have any idea how this could be done?

solnic commented 6 years ago

From @flash-gordon on June 27, 2017 11:11

🤔 @solnic what if we join base relations by default? I.e. users has many visible_comments, where visible_comments is a view defined to comments, then we just do join on users.id = comments.user_id, no matter how visible_comments is defined (user_id might not present in visible_comments we should raise an error in this case, I think it's fine for now). And in other cases we could leverage the block syntax you proposed in the chat

solnic commented 6 years ago

@flash-gordon I'm not sure if I follow :)

solnic commented 6 years ago

From @flash-gordon on June 28, 2017 11:18

@solnic lol, let's put it in another form, what's the problem with joining a view? RDMS has a similar concept of a "database view" and it's absolutely legal to join a view. We can do something very similar by, say, joining a subquery. WDYT?

solnic commented 6 years ago

ah yeah, this makes sense