Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Ignore binds payload with nil column in AR log subscriber #8838

Merged
merged 1 commit into from

3 participants

@yahonda

This pull request backports commit 77516a7 to 3-2-stable branch.

Conflict resolution:

  • Revert ruby 1.9 style hash to support ruby 1.8
  • Do not include 8f59ffc into 3-2-stable

This commit works with ruby-1.9.3-p362 without any regressions.
But it causes two RuntimeError: ActiveRecord::Associations::Association errors at test_sqlite3 when tested with ruby-1.8.7-p371.

It's strange for me, Tested 3-2-stable branch without my fix, a minimum testcase causes the same RuntimeError: ActiveRecord::Associations::Association.

@yahonda yahonda Ignore binds payload with nil column in AR log subscriber
Some tests were raising the following error:

    Could not log "sql.active_record" event. NoMethodError: undefined method
    `type' for nil:NilClass`

Due to the way binds were being logged, the column info was considered
always present, but that is not true for some of the tests listed in the
issue.

Closes #8806.

Conflicts:

	activerecord/lib/active_record/log_subscriber.rb
	activerecord/test/cases/log_subscriber_test.rb

Conflict resolution:
- Revert ruby 1.9 style hash to support ruby 1.8
- Do not include 8f59ffc into 3-2-stable
3d1a879
@rafaelfranca
Owner

@yahonda all green in my machine for the sqlite3 tests. I'll check the full Active Record suite. If all green I'll merge and see how the CI behaves.

@rafaelfranca rafaelfranca merged commit 3d1a879 into from
@yahonda

CI https://travis-ci.org/rails/rails/jobs/4043106 are all green. Thanks for testing and merging!

@yahonda yahonda deleted the branch
@parameme parameme referenced this pull request in dejan/rails_panel
Closed

SystemStackError (stack level too deep) #34

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 8, 2013
  1. @yahonda

    Ignore binds payload with nil column in AR log subscriber

    yahonda authored
    Some tests were raising the following error:
    
        Could not log "sql.active_record" event. NoMethodError: undefined method
        `type' for nil:NilClass`
    
    Due to the way binds were being logged, the column info was considered
    always present, but that is not true for some of the tests listed in the
    issue.
    
    Closes #8806.
    
    Conflicts:
    
    	activerecord/lib/active_record/log_subscriber.rb
    	activerecord/test/cases/log_subscriber_test.rb
    
    Conflict resolution:
    - Revert ruby 1.9 style hash to support ruby 1.8
    - Do not include 8f59ffc into 3-2-stable
This page is out of date. Refresh to see the latest.
View
6 activerecord/lib/active_record/log_subscriber.rb
@@ -32,7 +32,11 @@ def sql(event)
unless (payload[:binds] || []).empty?
binds = " " + payload[:binds].map { |col,v|
- [col.name, v]
+ if col
+ [col.name, v]
+ else
+ [nil, v]
+ end
}.inspect
end
View
34 activerecord/test/cases/log_subscriber_test.rb
@@ -7,6 +7,19 @@ class LogSubscriberTest < ActiveRecord::TestCase
include ActiveSupport::LogSubscriber::TestHelper
include ActiveSupport::BufferedLogger::Severity
+ class TestDebugLogSubscriber < ActiveRecord::LogSubscriber
+ attr_reader :debugs
+
+ def initialize
+ @debugs = []
+ super
+ end
+
+ def debug message
+ @debugs << message
+ end
+ end
+
fixtures :posts
def setup
@@ -32,18 +45,7 @@ def set_logger(logger)
def test_schema_statements_are_ignored
event = Struct.new(:duration, :payload)
- logger = Class.new(ActiveRecord::LogSubscriber) {
- attr_accessor :debugs
-
- def initialize
- @debugs = []
- super
- end
-
- def debug message
- @debugs << message
- end
- }.new
+ logger = TestDebugLogSubscriber.new
assert_equal 0, logger.debugs.length
logger.sql(event.new(0, { :sql => 'hi mom!' }))
@@ -56,6 +58,14 @@ def debug message
assert_equal 2, logger.debugs.length
end
+ def test_ignore_binds_payload_with_nil_column
+ event = Struct.new(:duration, :payload)
+
+ logger = TestDebugLogSubscriber.new
+ logger.sql(event.new(0, :sql => 'hi mom!', :binds => [[nil, 1]]))
+ assert_equal 1, logger.debugs.length
+ end
+
def test_basic_query_logging
Developer.all
wait
Something went wrong with that request. Please try again.