Unnecessarily slow PostgreSQLAdapter#columns #398

Closed
jaroslawr opened this Issue May 5, 2011 · 1 comment

2 participants

@jaroslawr

The PostgreSQLAdapter's "columns" method creates a new PostgreSQLColumn object for every column of a given database table. However, the constructor for this class has this form:

def initialize(name, default, sql_type = nil, null = true)
  super(name, self.class.extract_value_from_default(default), sql_type, null)
end

The 'default' parameter is a string describing a SQL data type whenever the DB column has a default value, however it is nil all the other times, that is: most of the time. Now, the extract_value_from_default method looks like this:

def self.extract_value_from_default(default)
  case default
    # Numeric types
    when /\A\(?(-?\d+(\.\d*)?\)?)\z/
      $1
    # Character types
    when /\A'(.*)'::(?:character varying|bpchar|text)\z/m
      $1
   ....
  end
end

Unfortunately it doesn't check for a nil anywhere, so it ends up calling === against nil for every 'when' clause in this method. It ends up causing a major performance issue especially:

a) in the development environment where this is called on every request
b) for complex schemas where there are many tables/columns

In my case, it made every request in development mode twice as slow. The same issue may affect other DB adapters. It doesn't affect ActiveRecord 2.3.10, and I think the reason for this might be the difference between NilClass#method_missing implementation between ActiveSupport 2.3.10:

def method_missing(method, *args, &block)
  # Ruby 1.9.2: disallow explicit coercion via method_missing.
  if method == :to_ary || method == :to_str
    super
  elsif klass = METHOD_CLASS_MAP[method]
    raise_nil_warning_for klass, method, caller
  else
    super
  end
end

And ActiveSupport 3.0.6:

def method_missing(method, *args, &block)
  if klass = METHOD_CLASS_MAP[method]
    raise_nil_warning_for klass, method, caller
  else
    super
  end
end

This fix for Ruby 1.9.2 was introduced by Jeremy Kemper in this commit: 9acc824 and I don't know what happened with it since and why.

An easy fix would be to modify PostgreSQLAdapter#initialize like this:

def initialize(name, default, sql_type = nil, null = true)
  default_value = default.present? ? self.class.extract_value_from_default(default) : nil
  super(name, default_value, sql_type, null)
end

I am on Ruby 1.9.2.

@jeremy
Ruby on Rails member

Coincidentally, this was recently fixed with 58ad5e1 :)

@jeremy jeremy closed this May 5, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment