Problem inferring a primary key when using aggregate

[mrship]

I have a primary key defined in my database schema for two tables. However, using either schema inference, or an explicit id with Types::Int - then I get the error below when using aggregate.

/.rbenv/versions/2.2.5/lib/ruby/gems/2.2.0/gems/sqlite3-1.3.13/lib/sqlite3/database.rb:91:in `initialize': SQLite3::SQLException: near "NULL": syntax error (Sequel::DatabaseError)

The code below demonstrates the problem. If the inferred schema is not used and the explicit version is used instead (the commented section) it also fails.

The only way to get this to work is to use, attribute :id, Types::Int.meta(primary_key: true) for both schemas.

This may be pilot error, but it was very confusing - and appears to be a breaking change with rom-sql 0.9.

Breaking example:

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

module Relations
  class Users < ROM::Relation[:sql]
    schema(:users) do
      attribute :id, Types::Int
      attribute :country, Types::String
    end
  end

  class Participants < ROM::Relation[:sql]
    # schema(:participants) do
    #   attribute :id, Types::Int
    #   attribute :name, Types::String
    #   attribute :user_id, Types::Int.meta(foreign_key: true, relation: :users)

    #   associations do
    #     belongs_to :user
    #   end
    # end
    dataset(:participants)
    schema(infer: true) do
      associations do
        belongs_to :user
      end
    end
  end
end

module Repositories
  class Participant < ROM::Repository[:participants]
    relations :users

    def all
      aggregate(:user).to_a
    end
  end
end

config = ROM::Configuration.new(:sql, "sqlite::memory")
config.register_relation Relations::Users
config.register_relation Relations::Participants
container = ROM.container(config)

container.gateways[:default].tap do |gateway|
  migration = gateway.migration do
    change do
      create_table :users do
        primary_key :id
        string :country, null: true
      end

      create_table :participants do
        primary_key :id
        string :name, null: true
        foreign_key :user_id, :users, null: false
      end
    end
  end
  migration.apply gateway.connection, :up
end

connection = container.gateways[:default].connection
connection.execute "INSERT INTO users (country) VALUES ('UK')"
connection.execute "INSERT INTO participants (name, user_id) VALUES ('Fred', 1)"

repo = Repositories::Participant.new(container)

repo.all

[solnic]

It works for me, I just had to fix you repro script because you create tables AFTER setting up rom, so inferrer doesn't see any tables. I'll add a warning to inferrer so that people can see that this happens. Here's a working script:

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

module Relations
  class Users < ROM::Relation[:sql]
    schema(:users, infer: true)
  end

  class Participants < ROM::Relation[:sql]
    schema(:participants, infer: true) do
      associations do
        belongs_to :user
      end
    end
  end
end

module Repositories
  class Participant < ROM::Repository[:participants]
    relations :users

    def all
      aggregate(:user).to_a
    end
  end
end

config = ROM::Configuration.new(:sql, "sqlite::memory")

config.gateways[:default].tap do |gateway|
  migration = gateway.migration do
    change do
      create_table :users do
        primary_key :id
        string :country, null: true
      end

      create_table :participants do
        primary_key :id
        string :name, null: true
        foreign_key :user_id, :users, null: false
      end
    end
  end
  migration.apply gateway.connection, :up
end

config.register_relation Relations::Users
config.register_relation Relations::Participants
container = ROM.container(config)

connection = container.gateways[:default].connection
connection.execute "INSERT INTO users (country) VALUES ('UK')"
connection.execute "INSERT INTO participants (name, user_id) VALUES ('Fred', 1)"

repo = Repositories::Participant.new(container)

puts repo.all.inspect
# [#<ROM::Struct[Participant] id=1 name="Fred" user_id=1 user=#<ROM::Struct[User] id=1 country="UK" participant_id=1>>]

[solnic]

@mrship please confirm that this was only a mistake in your setup, and not an actual bug in inferrer :)

[mrship]

Thanks for looking into this.

In an attempt to show the problem in this issue, my example was a little different to what we actually do in our app. Below is more in line with what we do with our app where we yield the container before we call auto_registration for our relations. The yield is used in the test suite to setup the database we run the tests against, so again, this is a slightly contrived example as we don't actually create tables each time we run the app (just the test suite). However, this does show creating the tables first then autoloading the relations.

I've got an issue with running this locally due to inflection and ROM vs Rom which I'll review to see if I can fix, but once I can this will give a clearer example of what we're actually doing.

Again, if this is pilot error and there are better ways of doing this then please point out the error of my ways :)

#!/usr/bin/env ruby
require "rom"
require "rom-sql"
require "rom-repository"

require_relative "./relations/users"
require_relative "./relations/participants"

config = ROM::Configuration.new(:sql, "sqlite::memory") do |container|
  container.config.gateways[:default].infer_relations = false

  connection = container[:default].connection
  connection.create_table(:users) do
    primary_key :id
    string :country, null: true
  end

  connection.create_table :participants do
    primary_key :id
    string :name, null: true
    foreign_key :user_id, :users, null: false
  end

  connection.execute "INSERT INTO users (country) VALUES ('UK')"
  connection.execute "INSERT INTO participants (name, user_id) VALUES ('Fred', 1)"

  container.auto_registration(__dir__)
end

module Repositories
  class Participant < ROM::Repository[:participants]
    relations :users

    def all
      aggregate(:user).to_a
    end
  end
end

container = ROM.container(config)
repo = Repositories::Participant.new(container)
repo.all

./relations/participants

require "dry-types"

module Relations
  class Participants < ROM::Relation[:sql]
    schema(:participants) do
      attribute :id, Types::Int.meta(primary_key: true)
      attribute :name, Types::String
      attribute :user_id, Types::Int.meta(foreign_key: true, relation: :users)

      associations do
        belongs_to :user
      end
    end
  end
end

./relations/users

require "dry-types"

module Relations
  class Users < ROM::Relation[:sql]
    schema(:users) do
      attribute :id, Types::Int.meta(primary_key: true)
      attribute :country, Types::String
    end
  end
end

[solnic]

I pushed this here https://github.com/solnic/rom-sql-issue-137

works for me, could you check? just clone + bundle install + bundle exec ruby rom.rb

[mrship]

Yes, that works for me too. Let's consider this closed then and I'll have a look at why this is/was failing in my actual app.

Thanks again for the support and best of luck with the RC cycle!

[mrship]

One last play with this:

class Users < ROM::Relation[:sql]
  schema(:users) do
    attribute :id, Types::Int
    attribute :country, Types::String
  end
end

If I don't use schema inference and specify Types::Int then it does still break, but in a different (and odd) way:

/Users/shipmana/.rbenv/versions/2.2.5/lib/ruby/gems/2.2.0/gems/dry-struct-0.1.1/lib/dry/struct/class_interface.rb:56:in `check_schema_duplication': Attribute :id has already been defined (Dry::Struct::RepeatedAttributeError)

I can work around that using Types::Serial but I thought I'd pass that on in case it is also a regression.

[solnic]

@mrship yeah that's certainly not the best way of handling a missing PK attribute. I'll improve this somehow. You need to either use inference, or do Types::Int.meta(primary_key: true) or primary_key :id.