Skip to content
Commits on Nov 23, 2012
  1. @jonleighton

    Use separate Relation subclasses for each AR class

    jonleighton committed
    At present, ActiveRecord::Delegation compiles delegation methods on a
    global basis. The compiled methods apply to all subsequent Relation
    instances. This creates several problems:
    
    1) After Post.all.recent has been called, User.all.respond_to?(:recent)
       will be true, even if User.all.recent will actually raise an error due
       to no User.recent method existing. (See #8080.)
    
    2) Depending on the AR class, the delegation should do different things.
       For example, if a Post.zip method exists, then Post.all.zip should call
       it. But this will then result in User.zip being called by a subsequent
       User.all.zip, even if User.zip does not exist, when in fact
       User.all.zip should call User.all.to_a.zip. (There are various
       variants of this problem.)
    
    We are creating these compiled delegations in order to avoid method
    missing and to avoid repeating logic on each invocation.
    
    One way of handling these issues is to add additional checks in various
    places to ensure we're doing the "right thing". However, this makes the
    compiled methods signficantly slower. In which case, there's almost no
    point in avoiding method_missing at all. (See #8127 for a proposed
    solution which takes this approach.)
    
    This is an alternative approach which involves creating a subclass of
    ActiveRecord::Relation for each AR class represented. So, with this
    patch, Post.all.class != User.all.class. This means that the delegations
    are compiled for and only apply to a single AR class. A compiled method
    for Post.all will not be invoked from User.all.
    
    This solves the above issues without incurring significant performance
    penalties. It's designed to be relatively seamless, however the downside
    is a bit of complexity and potentially confusion for a user who thinks
    that Post.all and User.all should be instances of the same class.
    
    Benchmark
    ---------
    
    require 'active_record'
    require 'benchmark/ips'
    
    class Post < ActiveRecord::Base
      establish_connection adapter: 'sqlite3', database: ':memory:'
      connection.create_table :posts
    
      def self.omg
        :omg
      end
    end
    
    relation = Post.all
    
    Benchmark.ips do |r|
      r.report('delegation')   { relation.omg }
      r.report('constructing') { Post.all }
    end
    
    Before
    ------
    
    Calculating -------------------------------------
              delegation      4392 i/100ms
            constructing      4780 i/100ms
    -------------------------------------------------
              delegation   144235.9 (±27.7%) i/s -     663192 in   5.038075s
            constructing   182015.5 (±21.2%) i/s -     850840 in   5.005364s
    
    After
    -----
    
    Calculating -------------------------------------
              delegation      6677 i/100ms
            constructing      6260 i/100ms
    -------------------------------------------------
              delegation   166828.2 (±34.2%) i/s -     754501 in   5.001430s
            constructing   116575.5 (±18.6%) i/s -     563400 in   5.036690s
    
    Comments
    --------
    
    Bear in mind that the standard deviations in the above are huge, so we
    can't compare the numbers too directly. However, we can conclude that
    Relation construction has become a little slower (as we'd expect), but
    not by a huge huge amount, and we can still construct a large number of
    Relations quite quickly.
Commits on Nov 22, 2012
  1. @rafaelfranca

    Merge pull request #8291 from senny/8265_build_with_polymorphic_assoc…

    rafaelfranca committed
    …iation
    
    prevent mass assignment of polymorphic type when using `build`
    
    Conflicts:
    	activerecord/CHANGELOG.md
  2. @carlosantoniodasilva
  3. @pixeltrix

    Merge pull request #8114 from guilleiguaran/use-symbols-in-scope

    pixeltrix committed
    Allow setting a symbol as path in scope on routes
  4. @senny
  5. @guilleiguaran

    Allow setting a symbol as path in scope on routes

    guilleiguaran committed
    Was surprising found that this example doesn't work:
    
      scope :api do
        resources :users
      end
    
    and the right form to use it is:
    
      scope 'api' do
        resources :users
      end
    
    I think this should work similary as `namespace` where both are allowed.
    These two are equivalent:
    
      namespace :api do
        resources :users
      end
    
      namespace 'api' do
        resources :user
      end
  6. @carlosantoniodasilva

    Remove private partial/template renderer methods

    carlosantoniodasilva committed
    Since now these objects are not cached anymore, there's no need to have
    these private methods, just instantiate each of them in the necessary
    place.
  7. @carlosantoniodasilva

    Merge branch 'deprecate-calculations-with-block'

    carlosantoniodasilva committed
    Follow up of the discussion from the original merge commit:
    f9cb645#commitcomment-1414561
    
    We want to avoid people's mistakes with methods like count and sum when
    called with a block, that can easily lead to code performing poorly and
    that could be way better written with a db query.
    
    Please check the discussion there for more background.
    Closes #8268
  8. @carlosantoniodasilva

    Remove the #sum method from CollectionAssociation

    carlosantoniodasilva committed
    Since edd94ce, CollectionProxy
    delegates all calculation methods - except count - to the scope,
    which does basically what this method was doing, but since we're
    delegating from the proxy, the association method was never called.
  9. @carlosantoniodasilva

    Deprecate Relation#sum with a block.

    carlosantoniodasilva committed
    To perform a sum calculation over the array of elements, use to_a.sum(&block).
    
    Please check the discussion in f9cb645
    for more context.
  10. @carlosantoniodasilva

    Revert "Yield only one argument instead of splatting."

    carlosantoniodasilva committed
    This reverts commit f9cb645.
    
    Conflicts:
    	activerecord/CHANGELOG.md
    
    Revert "Allow blocks for count with ActiveRecord::Relation. Document and test that sum allows blocks"
    
    This reverts commit 9cc2bf6.
    
    Conflicts:
    	activerecord/lib/active_record/relation/calculations.rb
Commits on Nov 21, 2012
  1. @jonleighton

    Don't allocate new strings in compiled attribute methods

    jonleighton committed
    This improves memory and performance without having to use symbols which
    present DoS problems. Thanks @headius and @tenderlove for the
    suggestion.
    
    This was originally committed in
    f176501, and then reverted in
    d349490 due to it causing problems in a
    real application. This second attempt should solve that.
    
    Benchmark
    ---------
    
    require 'active_record'
    require 'benchmark/ips'
    
    ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
    
    class Post < ActiveRecord::Base
      connection.create_table :posts, force: true do |t|
        t.string :name
      end
    end
    
    post = Post.create name: 'omg'
    
    Benchmark.ips do |r|
      r.report('Post.new')          { Post.new name: 'omg' }
      r.report('post.name')         { post.name }
      r.report('post.name=')        { post.name = 'omg' }
      r.report('Post.find(1).name') { Post.find(1).name }
    end
    
    Before
    ------
    
    Calculating -------------------------------------
                Post.new      1419 i/100ms
               post.name      7538 i/100ms
              post.name=      3024 i/100ms
       Post.find(1).name       243 i/100ms
    -------------------------------------------------
                Post.new    20637.6 (±12.7%) i/s -     102168 in   5.039578s
               post.name  1167897.7 (±18.2%) i/s -    5186144 in   4.983077s
              post.name=    64305.6 (±9.6%) i/s -     317520 in   4.998720s
       Post.find(1).name     2678.8 (±10.8%) i/s -      13365 in   5.051265s
    
    After
    -----
    
    Calculating -------------------------------------
                Post.new      1431 i/100ms
               post.name      7790 i/100ms
              post.name=      3181 i/100ms
       Post.find(1).name       245 i/100ms
    -------------------------------------------------
                Post.new    21308.8 (±12.2%) i/s -     105894 in   5.053879s
               post.name  1534103.8 (±2.1%) i/s -    7634200 in   4.979405s
              post.name=    67441.0 (±7.5%) i/s -     337186 in   5.037871s
       Post.find(1).name     2681.9 (±10.6%) i/s -      13475 in   5.084511s
  2. @carlosantoniodasilva
  3. @carlosantoniodasilva

    Use secure password min cost option in its own tests for a speed up

    carlosantoniodasilva committed
    Around 0.564359s => 0.092244s speed up in my machine.
  4. @jonleighton

    Merge pull request #8183 from jcoglan/objectless_sessions

    jonleighton committed
    Store FlashHashes in the session as plain hashes
  5. @rafaelfranca

    Merge pull request #7716 from steveklabnik/issue_7715

    rafaelfranca committed
    Coerce strings in create_join_table.
  6. @steveklabnik

    Coerce strings in create_join_table.

    steveklabnik committed
    If you accidentally pass a string and a symbol, this breaks. So
    we coerce them both to strings.
    
    Fixes #7715
  7. @carlosantoniodasilva
  8. @tuzz

    Typo

    tuzz committed
  9. @rafaelfranca

    Merge pull request #6245 from bogdan/bc_timestamp

    rafaelfranca committed
    Postgresql adapter: fix handling of BC timestamps
  10. @rafaelfranca

    Merge pull request #8289 from semaperepelitsa/pg_adapter_refactoring_…

    rafaelfranca committed
    …squashed
    
    Refactoring, testing and documenting pg_connection.distinct
  11. @semaperepelitsa
  12. @bogdan
  13. @rafaelfranca
Commits on Nov 20, 2012
  1. @rafaelfranca

    Merge pull request #8280 from asanghi/fix_guide_field_with_error_proc

    rafaelfranca committed
    fix guide with field_with_error proc example
  2. @carlosantoniodasilva

    Merge pull request #8282 from arunagw/warning_removed_for_ruby2

    carlosantoniodasilva committed
    Initialize accessors to remove some warnings in Ruby 2.0
  3. @arunagw
  4. @rafaelfranca

    Merge pull request #8276 from pwnall/pgsql_text_limits

    rafaelfranca committed
    Postgresql doesn't accepts limits on text columns
  5. @pwnall
  6. @asanghi
  7. @carlosantoniodasilva

    Merge pull request #8279 from gaurish/database_error

    carlosantoniodasilva committed
    Improved Error handling when parsing database.yaml, Fixes #8143
  8. @gaurish

    Better Error handling when parsing database.yaml

    gaurish committed
    Provides a better error message incase the database.yaml
    has some errors.
Commits on Nov 19, 2012
  1. @carlosantoniodasilva

    Merge branch 'url-for-params'

    carlosantoniodasilva committed
    Fix error when using a non-hash query argument named "params" in `url_for`.
    
    Before:
    
        url_for(params: "") # => undefined method `reject!' for "":String
    
    After:
    
        url_for(params: "") # => http://www.example.com?params=`
    
    Closes #8233
  2. @carlosantoniodasilva

    Correct the use of params options when given to url_for

    carlosantoniodasilva committed
    Merge url for tests and add changelog entry for #8233.
  3. @tumayun @carlosantoniodasilva

    Fix issue with params in url_for

    tumayun committed with carlosantoniodasilva
    With a "params" argument, the following error is raised:
    
        undefined method `reject!` for "":String
Something went wrong with that request. Please try again.