ActiveRecord models based on a view create new instances with id = 0 rather than nil #5982

Closed
stevenchanin opened this Issue Apr 25, 2012 · 29 comments

Comments

Projects
None yet
Contributor

stevenchanin commented Apr 25, 2012

Given a model like:

class Manufacturer < ActiveRecord::Base
  attr_accessible :name

  self.table_name = 'v_manufacturers'
  self.primary_key = 'id'
end

when you create a new instance

m = Manufacturer.new(name: 'Nissan')
# => #<Manufacturer id: 0, name: "Nissan">

but the id should be nil.

For a test case with tests (1.9.3 @ rails 3.2), see

https://github.com/stevenchanin/view_test

tenderlove was assigned May 29, 2012

Contributor

dmathieu commented May 30, 2012

Your database schema (db/schema.rb) looks like this :

create_table "v_manufacturers", :id => false, :force => true do |t|
  t.integer "id",   :default => 0, :null => false
end

The id has a default value of 0. Therefore, it's not nil in the object.

This file doesn't seem to be linked to your database migrations. Have you created it manually ?
If so, you should not ! db/schema.rb is generated automatically by rails when running the migrations.
If you need to change the schema of your database, you need to add a new migration which does the change.

arunagw closed this May 30, 2012

Contributor

stevenchanin commented May 31, 2012

I did not make any manual changes to schema.rb. Whatever is there was automatically generated by Rails as part of migrations, preparing the test db, or whatever. Could that be how Rails exports a View when it's in it's default schema dumper mode?

tenderlove reopened this May 31, 2012

Member

arunagw commented May 31, 2012

Sorry for closing this. Thanks Aaron for reopen this again

Contributor

vrish88 commented Jun 30, 2012

@stevenchanin Yeah, when rails dumps a view to schema.rb it dumps the schema in the block of the create_table method. It is mistreating a view as a table because the mysql adapter reports views as tables. Then, for some reason, mysql adds a default value of 0 on int columns in views.

The underlying issue here is how rails treats database views. Right now the database adapters treat views like tables. The mysql adapter issues SHOW TABLES; which will show views and tables and the postgres adapter issues:

SELECT tablename
FROM pg_tables
WHERE schemaname = ANY (current_schemas(false))

which includes views as well.

So when we dump the schema from the database to schema.rb, a view's schema will be placed within a create_table block, like in the comment above.

Rails is incorrectly representing views as tables in schema.rb, however I'm not sure what the appropriate resolution is.

How should views be represented in schema.rb?

Contributor

stevenchanin commented Jun 30, 2012

I took a look at the link you provided for mysql's show table command. There is a second command (show full tables) that provides a second column which identifies things as either "BASE TABLE"s or "VIEW"s. With PG, you can select from pg_views instead to get a list of views.

Given those two things, it should be possible to distinguish between views and tables, so we get back to your question about how to represent views...

Which I think would have to involve creating a new method (create_view) in ActiveRecord::ConnectionAdapters::SchemaStatements

Just off the top of my head, the usage could look like:

create_view 'v_manufacturers' do |v|
  v.source_table 'Manufacturers'
  v.column :id, :id
  v.column :name, :name

  #this wouldn't be used in my example, but ...
  v.column :calculated_value, '... some sql expression ...'
end

but I don't know how to get the info out of the databases to generate the command I proposed above

Contributor

vrish88 commented Jun 30, 2012

Perhaps we should use a simpler interface when creating a view. There will be some cases where it's a very simple case with a select statement that can be transfered between different types of databases:

create_view 'v_manufacturers' do |v|
  v.columns = [:id, :name]
  v.as_sql = Manufacturers.select([:id, :name]) # can also pass a string of sql
end

But I also think we should give people the ability to create views with the full sql so database specific parameters can be incorporated. For instance, if no block is passed in we can assume the first argument is the sql needed to create the view:

create_view 'CREATE SQL SECURITY DEFINER VIEW v_manufacturers(id, name) AS SELECT manufacturer_id, manufacturer_name FROM Manufacturers'
Contributor

stevenchanin commented Jul 2, 2012

I think your proposal makes a lot of sense.

In 100% of the cases where I've used views in my Rails app it has been as a way to insert a layer that makes naming of an existing table conform to Rails conventions (where the underlying columns are called "manufacturer_name", but I want them to simply be "name") but where I can't simply create a new table and migrate the data because some legacy app (e.g. PHP code, etc) has references to the existing table / schema.

In all of those instances, views have only pointed to 1 table ("v_manufacturers" ==> "Manufacturers") and there haven't been any calculated columns. In part, I've done that so that updates to the view can be processed through by the database to the underlying table.

The thing I like about sticking a view in (as described above) is that I can write idiomatic Rails code

@manufacturer.name

and

@manufacturer.update_attributes(:name => "Ford")

and have it work through the view. But then downstream when we retire the legacy code, then I can create an actual "manufacturers" table with columns actually called "name", migrate the data across

INSERT into manufacturers(id,name) SELECT(manufacturer_id, manufacturer_name) from Manufacturers

and drop the view and the legacy table (or just do a bunch of renames) and my Rails app is unchanged

Contributor

parndt commented Dec 13, 2012

I can confirm that this is still occurring on Rails 3.2.9. I updated the Gemfile in the view_test application to 3.2.9 and tested using:

rm db/schema.rb
bundle exec rake db:drop db:create db:migrate db:test:prepare
bundle exec rake test
cat db/schema.rb

Still seeing :id => 0:

ActiveRecord::Schema.define(:version => 20120425202029) do

  create_table "Manufacturers", :primary_key => "manufacturer_id", :force => true do |t|
    t.string "manufacturer_name"
  end

  create_table "cars", :force => true do |t|
    t.string   "name"
    t.integer  "manufacturer_id"
    t.datetime "created_at",      :null => false
    t.datetime "updated_at",      :null => false
  end

  add_index "cars", ["manufacturer_id"], :name => "index_cars_on_manufacturer_id"

  create_table "v_manufacturers", :id => false, :force => true do |t|
    t.integer "id",   :default => 0, :null => false
    t.string  "name"
  end

end

And failing that test:

# Running tests:

.F

Finished tests in 0.028802s, 69.4396 tests/s, 69.4396 assertions/s.

  1) Failure:
test_a_new_manufacturer_should_have_id_=_nil(ManufacturerTest) [/Users/parndt/sandbox/view_test/test/unit/manufacturer_test.rb:5]:
Failed assertion, no message given.

jwg2s commented Dec 19, 2012

We're also using views exactly like @stevenchanin describes in his most recent comment. We have no way of getting the most recently created object's id back and it returns id: 0.

Definitely a massive problem when trying to develop a new rails application alongside a legacy app that relies on data in a certain format until it can all be migrated after a new app takeover.

What do you guys do to get around this?

jwg2s commented Dec 21, 2012

One potential solution to this is to use UUID columns (for models where there really is no other unique characteristic) and lookup the object you just created using the UUID.

This obviously isn't what I want to be doing, so any other suggestions here would be awesome.

Owner

tenderlove commented Jan 19, 2013

So it seems as if the real bug is that the generated schema.rb is incorrect. Let's look at fixing that.

eugenekorpan referenced this issue in activebridge/multitenant-mysql Feb 15, 2013

Closed

Problem with record creation thru the view #1

I just wanted to pitch in that we've been using views for the same purpose, but have not had this issue until rails 3.2

It DEFINITELY wasn't there is 3.0.20, it could have been there in 3.1.11 (we jumped quickly at this point), and it has now stonewalled us in 3.2.12.

Clearly the way the schema is generated has changed from 3.0 to 3.2.

Owner

pixeltrix commented Mar 7, 2013

As an interim fix we should probably skip views in schema.rb for 4.0 and then maybe look at supporting views for 4.1

orikremer referenced this issue in jruby/activerecord-jdbc-adapter May 17, 2013

Closed

fix views dump in mysql structure dump #385

Contributor

graysonwright commented Jul 27, 2013

Bump. It looks like 385 resolves this?

Member

senny commented Mar 26, 2014

@stevenchanin @pixeltrix is there something left to do here?

Member

senny commented Apr 23, 2014

I wrote a reproduction (https://gist.github.com/senny/11220560) and the test case is passing. Closing this for now. Let me know if there is anything left to do and I'll reopen.

senny closed this Apr 23, 2014

Contributor

stevenchanin commented Apr 23, 2014

Just looked at the gist and that looks great. Thanks for checking this and closing it out. I’m sorry I didn’t have time to be more help :-(

Thanks,
Steve


Steven Chanin
steven_chanin@alum.mit.edu
(650) 701-7003

On Apr 23, 2014, at 11:44 AM, Yves Senn notifications@github.com wrote:

I wrote a reproduction (https://gist.github.com/senny/11220560) and the test case is passing. Closing this for now. Let me know if there is anything left to do and I'll reopen.


Reply to this email directly or view it on GitHub.

eddiej commented Aug 27, 2015

The original test case, using the mysql2 adapter, is still failing. @stevenchanin did you manage to resolve this in another way? The reproduction above written by @senny and that closed the issue uses the sqlite3 adapter, this may be what is causing it to pass, or it could be how the database view is created - using ActiveRecord::Schema.define.

I've created a simplified test case using Rails 4 and Ruby 2.2.3:
https://github.com/eddiej/rails4_view_test
https://travis-ci.org/eddiej/rails4_view_test

Member

senny commented Aug 27, 2015

@eddiej just ran my test-case using mysql2 against master and it passes:

ActiveRecord::Base.establish_connection(adapter: 'mysql2', database: 'test123')

Can you reproduce the error modifying my gist?

Contributor

stevenchanin commented Aug 27, 2015

@eddiej I'm sorry, but I don't recall. This problem came up with a project to replace a PHP site with Rails. Ultimately, we got all the PHP code retired, switched to real tables (not views) that followed Rails naming conventions and then it was no longer an issue.

eddiej commented Aug 28, 2015

@senny if you create the Manufacturers table with an id primary key, and create the view to select this id (instead of using manufacturer_id), the test cases fail using the mysql2 adaptor:

https://gist.github.com/eddiej/28ad6fc04238348aca94

I've changed the assertion slightly to:

assert_equal(nil, Manufacturer.new.id)

The test result is:

  1) Failure:
BugTest#test_association_stuff [test.rb:42]:
Expected: nil
  Actual: 0
Contributor

stevenchanin commented Aug 28, 2015

@eddiej Nice job creating such a clean & simple test.

Member

senny commented Sep 7, 2015

@eddiej I'll have a look.

Member

senny commented Sep 7, 2015

@eddiej I was able to reproduce the issue. It's due to the fact that the model based on the view has a Column object with default=0. However digging deeper I found that this is what MySQL reports about the view. See the following snippet from the MySQL-CLI:

mysql> SHOW FULL FIELDS FROM Manufacturers;
+-------------------+--------------+-----------------+------+-----+---------+----------------+---------------------------------+---------+
| Field             | Type         | Collation       | Null | Key | Default | Extra          | Privileges                      | Comment |
+-------------------+--------------+-----------------+------+-----+---------+----------------+---------------------------------+---------+
| id                | int(11)      | NULL            | NO   | PRI | NULL    | auto_increment | select,insert,update,references |         |
| manufacturer_name | varchar(255) | utf8_unicode_ci | YES  |     | NULL    |                | select,insert,update,references |         |
+-------------------+--------------+-----------------+------+-----+---------+----------------+---------------------------------+---------+
2 rows in set (0.00 sec)

mysql> SHOW FULL FIELDS FROM v_manufacturers;
+-------+--------------+-----------------+------+-----+---------+-------+---------------------------------+---------+
| Field | Type         | Collation       | Null | Key | Default | Extra | Privileges                      | Comment |
+-------+--------------+-----------------+------+-----+---------+-------+---------------------------------+---------+
| id    | int(11)      | NULL            | NO   |     | 0       |       | select,insert,update,references |         |
| name  | varchar(255) | utf8_unicode_ci | YES  |     | NULL    |       | select,insert,update,references |         |
+-------+--------------+-----------------+------+-----+---------+-------+---------------------------------+---------+
2 rows in set (0.01 sec)

I don't think this is something that Active Record should thinker around. It's behaving as the database tells it to.

GerryG commented Apr 24, 2016

This isn't fixed for 3.2.x, is it?

Member

senny commented Apr 25, 2016 edited

@GerryG it's not. Also note that 3.2.x does non longer receive bug fixes by the Rails team.

GerryG commented Apr 25, 2016

I understand these versions are not under active support, but I was at least hoping there was a user patch or something. Seems like that would be hard and not worth it, so we may have to forgo using the view altogether.

Just to clarify, there is a way to handle it in some rails 4 version? That may be something we could do, but not immediately. It seems like the fact that mysql reports the column as default: 0 on the view would need to be fixed in mysql specific code. Is that so?

GerryG commented Apr 25, 2016

It isn't clear to me from the comments if this is fixed in any version of rails.

Hi, we've experienced this in our project recently and hopefully I've found an easy fix. We use MySQL and senny's comment was the start point for solving this, thanks =). For details see the comment here.

jonatanklosko referenced this issue in thewca/worldcubeassociation.org May 22, 2016

Merged

Port change person to rails #645

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment