Creating with a changeset fails with Oracle

[solnic]

From @mrship on January 30, 2017 12:39

Using a changeset to create a record is failing for me when using Oracle, because (I think) you don't get the result of the INSERT statement back from Oracle using Sequel.

This is my code:

changeset_for_create = changeset(participant.to_hash)
  .map(:add_timestamps)
  .associate(participant.user, :users)
create(changeset_for_create(participant))

Which fails with:

2017-01-30 12:28:42 - Dry::Struct::Error - [ROM::Struct[Participant].new] :id is missing in Hash input:
	/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:80:in `rescue in new'
	/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:74:in `new'
	/Users/shipmana/.rbenv/versions/2.2.5/lib/ruby/gems/2.2.0/gems/transproc-1.0.0/lib/transproc/class.rb:30:in `constructor_inject'
	/Users/shipmana/.rbenv/versions/2.2.5/lib/ruby/gems/2.2.0/gems/transproc-1.0.0/lib/transproc/function.rb:47:in `call'
	/Users/shipmana/.rbenv/versions/2.2.5/lib/ruby/gems/2.2.0/gems/transproc-1.0.0/lib/transproc/function.rb:47:in `call'
	/Users/shipmana/.rbenv/versions/2.2.5/lib/ruby/gems/2.2.0/gems/transproc-1.0.0/lib/transproc/array.rb:41:in `block in map_array'
	/Users/shipmana/.rbenv/versions/2.2.5/lib/ruby/gems/2.2.0/gems/transproc-1.0.0/lib/transproc/array.rb:41:in `map'
	/Users/shipmana/.rbenv/versions/2.2.5/lib/ruby/gems/2.2.0/gems/transproc-1.0.0/lib/transproc/array.rb:41:in `map_array'
	/Users/shipmana/.rbenv/versions/2.2.5/lib/ruby/gems/2.2.0/gems/transproc-1.0.0/lib/transproc/function.rb:47:in `call'
	/Users/shipmana/.rbenv/versions/2.2.5/lib/ruby/gems/2.2.0/gems/transproc-1.0.0/lib/transproc/function.rb:47:in `call'
	/Users/shipmana/.rbenv/versions/2.2.5/lib/ruby/gems/2.2.0/gems/rom-mapper-0.5.0/lib/rom/mapper.rb:95:in `block in call'
	/Users/shipmana/.rbenv/versions/2.2.5/lib/ruby/gems/2.2.0/gems/rom-mapper-0.5.0/lib/rom/mapper.rb:95:in `each'
	/Users/shipmana/.rbenv/versions/2.2.5/lib/ruby/gems/2.2.0/gems/rom-mapper-0.5.0/lib/rom/mapper.rb:95:in `reduce'
	/Users/shipmana/.rbenv/versions/2.2.5/lib/ruby/gems/2.2.0/gems/rom-mapper-0.5.0/lib/rom/mapper.rb:95:in `call'
	/Users/shipmana/.rbenv/versions/2.2.5/lib/ruby/gems/2.2.0/gems/rom-repository-1.0.0.rc1/lib/rom/repository.rb:326:in `map_tuple'
	/Users/shipmana/.rbenv/versions/2.2.5/lib/ruby/gems/2.2.0/gems/rom-repository-1.0.0.rc1/lib/rom/repository/class_interface.rb:117:in `block in define_command_method'

This works fine with sqlite (in a test) but not with our production database of Oracle. I have had a similar issue in the past when expecting create to return a ROM::Struct but it does not with Oracle because of the issue with Sequel not using RETURNING as part of the INSERT query. As I understand it Sequel returns false for supports_returning? for Oracle.

My workaround is not to use changesets at all...

attributes = participant.to_hash.merge(user_id: participant.user.id)
create(attributes)

which is a real shame as I like the interface they provide and the convenience of the associate method and adding timestamps etc.

Can changesets be made to work without expecting a result from the underlying INSERT?

Copied from original issue: rom-rb/rom-repository#55

[solnic]

From @flash-gordon on January 30, 2017 12:54

I bet that's because rom-sql does not use returning statement for Oracle. Thanks for reporting this, I'll have a look. Actually, I'm going to add more full-featured Oracle support later this month. It seems Oracle's licensing policy does allow us to have XE on Travis and it can be downloaded & installed in acceptable time. I found other OSS projects that do so.

[solnic]

There was 0 work done for Oracle so far, so it doesn't surprise me :/

[solnic]

From @mrship on January 30, 2017 20:12

In reading the rom 3.0 blog post I realised that I can use the commit method on changeset instead of using create.

changeset_for_create = changeset(participant.to_hash)
  .map(:add_timestamps)
  .associate(participant.user, :users)
changeset_for_create.commit

That's a practical workaround for me for now.

[solnic]

From @mrship on January 31, 2017 11:50

It turns out that using commit is not a practical workaround as it doesn't actually work.

Per the code below if you call #commit on an updated changeset, it actually updates every record, rather than the one specified by id.

I feel like I'm doing something wrong though because I can't credit that this is the expected behaviour?

#!/usr/bin/env ruby
require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'
  gem 'rom-sql', '~> 1.0'
  gem 'rom-repository', '~> 1.0'
  gem 'dry-types'
  gem 'sqlite3'
end

require 'rom-repository'
require 'rom-sql'
require 'dry-types'

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

config = ROM::Configuration.new(:sql, 'sqlite::memory') do |container|
  default = container.default

  default.create_table :participants do
    primary_key :id
    string :name, null: true
  end

  default.connection.execute "INSERT INTO participants (name) VALUES ('Fred')"
  default.connection.execute "INSERT INTO participants (name) VALUES ('Bob')"

  container.register_relation(Participants)
end

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

    def by_id(id)
      participants.by_pk(id).one
    end

    def update_name(participant, name)
      changeset(participant.id, participant.to_hash.merge(name: name)).commit
    end
  end
end

container = ROM.container(config)
repo = Repositories::Participant.new(container)
puts repo.by_id(1).inspect
puts repo.by_id(2).inspect

participant = repo.by_id(1)
repo.update_name(participant, "Jane")

puts repo.by_id(1).inspect
puts repo.by_id(2).inspect

#<ROM::Struct[Participant] id=1 name="Fred">
#<ROM::Struct[Participant] id=2 name="Bob">
#<ROM::Struct[Participant] id=1 name="Jane">
#<ROM::Struct[Participant] id=2 name="Jane">

[solnic]

@mrship seems like a weird and nasty bug, looking into it

[solnic]

@mrship OK I fixed it in 1.0.1 (released already)

[solnic]

From @mrship on January 31, 2017 12:29

Thanks for the quick response.

[solnic]

From @flash-gordon on February 12, 2017 22:22

I have some news about this. Sequel currently does not support RETURNING for Oracle. Instead it queries the sequence associated with the table for the current value after running the insert statement. It works so far, but if two conditions are met:

1. You passed autosequence: true with connection options.
2. Your sequences are called "seq_#{table}_#{primary_key}". And otherwise you're out of luck.

Actually, Sequel's connection object can be hacked by setting a proper value for @primary_key_sequences instance variable which maps tables and sequence names. I've no idea why Sequel does not offer a public API for this provided that adding it is obviously a piece of cake.

It's still not perfect, i.e. won't work for composite PKs and requires a separate query (though a very fast one), but should do the trick for you @mrship

I'll be adding support for RETURNING to Sequel, these manipulations with @primary_key_sequences is a temporary workaround.

[solnic]

@flash-gordon you do what you gotta do :) I don't use Oracle so I don't have much to say. Feel free to push a release once this is sorted out

[solnic]

From @mrship on February 13, 2017 17:37

@flash-gordon Thanks for looking at this. I had a similar issue when trying to use changesets on a table that has no primary key; again it fails because there is, er, no primary key.

I can insert records directly into one of these tables, and tbh I'm not sure if changesets can/should be made to work without a primary key in the table definition, but this was not an Oracle-specific bug as it fails on sqlite too.

Let me know if you think this is worth reporting as a separate issue. I saw your comments in gitter today that changesets expect a primary_key of id, so perhaps this is beyond the current implementation?

[solnic]

From @flash-gordon on February 14, 2017 13:58

@mrship I recently added a check for PK presence https://github.com/rom-rb/rom-sql/blob/40885890ef33f4c1cd23e71019a4a616985f7f0b/lib/rom/sql/relation.rb#L94-L97

[solnic]

From @flash-gordon on February 26, 2017 18:36

heh, it took me a while https://github.com/jeremyevans/sequel/pull/1312/files