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

Updating column on model without PK causes "undefined method 'to_sym' for nil:NilClass" #20755

Closed
hrenfroe opened this Issue Jul 2, 2015 · 9 comments

Comments

Projects
None yet
7 participants
@hrenfroe
Copy link

hrenfroe commented Jul 2, 2015

In 4.2.3, when I create a table without a primary key and try to update an instance (via touch or update_column) I receive the following exception:

NoMethodError: undefined method `to_sym' for nil:NilClass
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/sanitization.rb:59:in `block in expand_hash_conditions_for_aggregates'
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/sanitization.rb:58:in `each'
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/sanitization.rb:58:in `expand_hash_conditions_for_aggregates'
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/relation/query_methods.rb:957:in `build_where'
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/relation/query_methods.rb:584:in `where!'
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/relation/query_methods.rb:574:in `where'
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/persistence.rb:474:in `touch'
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/callbacks.rb:296:in `block in touch'
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activesupport-4.2.3/lib/active_support/callbacks.rb:84:in `run_callbacks'
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/callbacks.rb:296:in `touch'
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/transactions.rb:295:in `block in touch'
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/transactions.rb:351:in `block in with_transaction_returning_status'
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/connection_adapters/abstract/database_statements.rb:213:in `block in transaction'
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/connection_adapters/abstract/transaction.rb:184:in `within_new_transaction'
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/connection_adapters/abstract/database_statements.rb:213:in `transaction'
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/transactions.rb:220:in `transaction'
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/transactions.rb:348:in `with_transaction_returning_status'
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/transactions.rb:295:in `touch'
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/no_touching.rb:49:in `touch'
    bug_exec.rb:38:in `test_widget_touch'

This looks like it's related to #13893.

Executable test case:

begin
  require 'bundler/inline'
rescue LoadError => e
  $stderr.puts 'Bundler version 1.10 or later is required. Please update your Bundler'
  raise e
end

gemfile(true) do
  source 'https://rubygems.org'
  # Activate the gem you are reporting the issue against.
  gem 'activerecord', '4.2.3'
  gem 'sqlite3'
end

require 'active_record'
require 'minitest/autorun'
require 'logger'

# Ensure backward compatibility with Minitest 4
Minitest::Test = MiniTest::Unit::TestCase unless defined?(Minitest::Test)

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :widgets, id: false, force: true do |t|
    t.timestamp :updated_at
  end
end

class Widget < ActiveRecord::Base
end

class BugTest < Minitest::Test
  def test_widget_touch
    widget = Widget.create
    widget.touch
    assert_not_nil(widget)
  end
end
@fenec

This comment has been minimized.

Copy link
Contributor

fenec commented Jul 2, 2015

It never worked as touch uses a primary key to find the record for update.

@meinac

This comment has been minimized.

Copy link
Contributor

meinac commented Jul 3, 2015

Actually activerecord does not tries to find record in this case. It tries to generate where clause to update correct one and can't find primary_key for where clause.
What is the use case of touching the record which has any distinctive column? As you know you have to provide at least one decisive column to update the particular row in database otherwise it will update all rows.
In your case I recommend you to use a primary_key.
@rafaelfranca What do you think about the raising exception for explaining why touch requires primary_key?

@hrenfroe

This comment has been minimized.

Copy link
Author

hrenfroe commented Jul 3, 2015

My use case is a single-column, single-row table that just keeps a timestamp because there's no call for anything more in the app. However, the issue is not limited to touch, or single-column tables. update_column will throw the same exception:

begin
  require 'bundler/inline'
rescue LoadError => e
  $stderr.puts 'Bundler version 1.10 or later is required. Please update your Bundler'
  raise e
end

gemfile(true) do
  source 'https://rubygems.org'
  # Activate the gem you are reporting the issue against.
  gem 'activerecord', '4.2.3'
  gem 'sqlite3'
end

require 'active_record'
require 'minitest/autorun'
require 'logger'

# Ensure backward compatibility with Minitest 4
Minitest::Test = MiniTest::Unit::TestCase unless defined?(Minitest::Test)

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :widgets, id: false, force: true do |t|
    t.timestamp :updated_at
    t.string :name
  end
end

class Widget < ActiveRecord::Base
end

class BugTest < Minitest::Test
  def test_widget_touch
    widget = Widget.create
    widget.update_column(:name, 'foo')
    assert_not_nil(widget.name)
  end
end
NoMethodError: undefined method `to_sym' for nil:NilClass
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/sanitization.rb:59:in `block in expand_hash_conditions_for_aggregates'
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/sanitization.rb:58:in `each'
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/sanitization.rb:58:in `expand_hash_conditions_for_aggregates'
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/relation/query_methods.rb:957:in `build_where'
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/relation/query_methods.rb:584:in `where!'
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/relation/query_methods.rb:574:in `where'
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/persistence.rb:299:in `update_columns'
    /usr/local/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/persistence.rb:273:in `update_column'
    bug_exec.rb:39:in `test_widget_touch'

I don't mind using a primary key if it is needed. If we keep this behavior, I'd be happy with an explanatory exception.

@meinac

This comment has been minimized.

Copy link
Contributor

meinac commented Jul 3, 2015

@HWilbanks normally in SQL if you want to update only one row you need to provide a where clause with identifier column like this;

UPDATE foo SET bar=zoo WHERE id=1;

If you don't provide a where clause it will update whole table.
In your case you want to update the updated_at value of only one row and activerecord creates the where clause for you. But it can't find any selective attribute and this causes problem.
Under the hood touch just updates the updated_at attribute of the record with UPDATE query so it requires primary_key like update_column. I can say that primary_key is needed for these operations.

@senny senny added the activerecord label Jul 7, 2015

@senny

This comment has been minimized.

Copy link
Member

senny commented Aug 10, 2015

Active Record needs a primary key to find and update records. If for some reason you don't want a PK in your single column, single row table, you can use raw SQL for your use-cases.

@senny senny closed this Aug 10, 2015

@maikokuppe

This comment has been minimized.

Copy link

maikokuppe commented Nov 16, 2016

It would be great to get an error like 'This might be fixed by adding a PK to migration xyz' in such a case.

@senny

This comment has been minimized.

Copy link
Member

senny commented Nov 21, 2016

@maikokuppe I agree that the error could be more descriptive. I'll see what I can do.

@natebird

This comment has been minimized.

Copy link

natebird commented May 3, 2017

I ran into this issue this week as well. It took a little bit of effort to see that I needed a primary key to delete join table records. A better error would have gone a long way.

@krisleech

This comment has been minimized.

Copy link

krisleech commented Apr 4, 2018

Same here, this happened for a vanilla model without a primary key, i.e. the create_table migration has id: false, when doing update_attributes etc.

Adding self.primary_key = 'reference' fixed the issue.

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