1. 22 5月, 2019 1 次提交
  2. 21 5月, 2019 3 次提交
    • R
      Put all `explain` methods into `DatabaseStatements` module · 36f6327b
      Ryuta Kamizono 提交于
      Almost all database statements methods except `explain` was moved into
      `DatabaseStatements` at #35922. This moves the last one method.
      36f6327b
    • R
      Fall back to type casting from the connection adapter · 82a54be0
      Ryuta Kamizono 提交于
      Unfortunately, a11a8ff7 had no effect as long as using bind param, and
      was not tested.
      
      This ensures making the intent of a11a8ff7, which fall back to type
      casting from the connection adapter.
      
      Fixes #35205.
      
      ```
      % ARCONN=postgresql bundle exec ruby -w -Itest test/cases/relation/where_test.rb -n test_type_casting_nested_joins
      Using postgresql
      Run options: -n test_type_casting_nested_joins --seed 55730
      
      # Running:
      
      E
      
      Error:
      ActiveRecord::WhereTest#test_type_casting_nested_joins:
      ActiveRecord::StatementInvalid: PG::InvalidTextRepresentation: ERROR:  invalid input syntax for integer: "2-foo"
      
      rails test test/cases/relation/where_test.rb:30
      
      Finished in 0.245778s, 4.0687 runs/s, 0.0000 assertions/s.
      1 runs, 0 assertions, 0 failures, 1 errors, 0 skips
      ```
      82a54be0
    • R
      Except SCHEMA SQLs in `capture_sql` · 0810c076
      Ryuta Kamizono 提交于
      Testing the result of `capture_sql` is fragile, it is due to whether
      SCHEMA SQLs are executed or not depends on whether schema cache is
      filled or not.
      
      https://buildkite.com/rails/rails/builds/61248#a5b9dc59-ff0c-40c0-b56e-0895662fbc4c/993-1004
      https://buildkite.com/rails/rails/builds/61248#1157b389-f2c7-4554-b6e5-a37624a0e74a/996-1005
      
      I've confirmed all `capture_sql` use cases in our code base, all cases
      won't expect SCHEMA SQLs are included.
      
      ```
      % git grep -n capture_sql
      test/cases/associations/belongs_to_associations_test.rb:202:    sql = capture_sql { comment.post }
      test/cases/associations/belongs_to_associations_test.rb:204:    assert_not_equal sql, capture_sql { comment.post }
      test/cases/associations/has_many_associations_test.rb:169:    sql = capture_sql { post.comments.to_a }
      test/cases/associations/has_many_associations_test.rb:171:    assert_not_equal sql, capture_sql { post.comments.to_a }
      test/cases/associations/has_many_associations_test.rb:276:    expected_sql = capture_sql { author.thinking_posts.delete_all }
      test/cases/associations/has_many_associations_test.rb:281:    loaded_sql = capture_sql { author.thinking_posts.delete_all }
      test/cases/associations/has_many_associations_test.rb:289:    expected_sql = capture_sql { author.posts.delete_all }
      test/cases/associations/has_many_associations_test.rb:294:    loaded_sql = capture_sql { author.posts.delete_all }
      test/cases/associations/left_outer_join_association_test.rb:22:      queries = capture_sql do
      test/cases/associations/left_outer_join_association_test.rb:49:    queries = capture_sql { Author.left_outer_joins(:posts).to_a }
      test/cases/associations/left_outer_join_association_test.rb:54:    queries = capture_sql { Author.joins(:posts).left_outer_joins(:posts).to_a }
      test/cases/associations/left_outer_join_association_test.rb:60:    queries = capture_sql { Author.left_outer_joins({}).to_a }
      test/cases/associations/left_outer_join_association_test.rb:65:    queries = capture_sql { Author.left_outer_joins([]).to_a }
      test/cases/associations/left_outer_join_association_test.rb:78:    queries = capture_sql { Author.left_outer_joins(:essays).to_a }
      test/cases/associations_test.rb:384:    log = capture_sql do
      test/cases/associations_test.rb:399:    log = capture_sql do
      test/cases/associations_test.rb:414:    log = capture_sql do
      test/cases/associations_test.rb:429:    log = capture_sql do
      test/cases/associations_test.rb:444:    log = capture_sql do
      test/cases/associations_test.rb:459:    log = capture_sql do
      test/cases/reflection_test.rb:307:    expected_sql = capture_sql { hotel.recipes.to_a }
      test/cases/reflection_test.rb:312:    loaded_sql = capture_sql { hotel.recipes.to_a }
      test/cases/relation_test.rb:212:      queries = capture_sql { Author.joins(:posts).merge(Post.joins(:comments)).to_a }
      test/cases/relation_test.rb:232:      queries = capture_sql { Post.joins(:author, :categorizations).merge(Author.select(:id)).merge(categorizations_with_authors).to_a }
      test/cases/relation_test.rb:347:      log = capture_sql do
      test/cases/scoping/relation_scoping_test.rb:146:      log = capture_sql do
      test/cases/scoping/relation_scoping_test.rb:159:    log = capture_sql do
      test/cases/test_case.rb:33:    def capture_sql
      test/cases/test_case.rb:41:      capture_sql { yield }
      ```
      0810c076
  3. 19 5月, 2019 1 次提交
    • R
      Implicit through table joins should be appeared before user supplied joins · 7412b7f8
      Ryuta Kamizono 提交于
      #36293 was an issue for through association with `joins` for a long
      time, but since #35864 through association with `left_joins` would also
      be affected by the issue.
      
      Implicit through table joins should be appeared before user supplied
      joins, otherwise loading through association with joins will cause a
      statement invalid error.
      
      Fixes #36293.
      
      ```
      % ARCONN=postgresql bundle exec ruby -w -Itest test/cases/associations/has_many_through_associations_test
      .rb -n test_through_association_with_joins
      Using postgresql
      Run options: -n test_through_association_with_joins --seed 7116
      
      # Running:
      
      E
      
      Error:
      HasManyThroughAssociationsTest#test_through_association_with_joins:
      ActiveRecord::StatementInvalid: PG::UndefinedTable: ERROR:  missing FROM-clause entry for table "posts"
      LINE 1: ... "comments_posts" ON "comments_posts"."post_id" = "posts"."i...
                                                                   ^
      : SELECT "comments".* FROM "comments" INNER JOIN "comments" "comments_posts" ON "comments_posts"."post_id" = "posts"."id" INNER JOIN "posts" ON "comments"."post_id" = "posts"."id" WHERE "posts"."author_id" = $1
      
      rails test test/cases/associations/has_many_through_associations_test.rb:61
      
      Finished in 0.388657s, 2.5730 runs/s, 0.0000 assertions/s.
      1 runs, 0 assertions, 0 failures, 1 errors, 0 skips
      ```
      7412b7f8
  4. 18 5月, 2019 1 次提交
  5. 15 5月, 2019 1 次提交
  6. 13 5月, 2019 2 次提交
  7. 12 5月, 2019 2 次提交
  8. 07 5月, 2019 3 次提交
    • Y
      Remove ignored_sql from SQLCounter by adding "TRANSACTION" to log name · 6a32e8aa
      Yasuo Honda 提交于
      This commit adds "TRANSACTION" to savepoint and commit, rollback statements
      because none of savepoint statements were removed by #36153 since they are not "SCHEMA" statements.
      
      Although, only savepoint statements can be labeled as "TRANSACTION"
      I think all of transaction related method should add this label.
      
      Follow up #36153
      6a32e8aa
    • A
      Properly give defaults for DatabaseSelector options · cecbc234
      Akira Matsuda 提交于
      The initializer receives `nil` for these options when no cofigurations were given:
      https://github.com/rails/rails/blob/v6.0.0.rc1/activerecord/lib/active_record/railtie.rb#L91-L97
      cecbc234
    • R
      Should attempt `committed!`/`rolledback!` to all enrolled records in the transaction · 718a32ca
      Ryuta Kamizono 提交于
      Currently, `committed!`/`rolledback!` will only be attempted for the
      first enrolled record in the transaction, that will cause some
      problematic behaviors.
      
      The first one problem, `clear_transaction_record_state` won't be called
      even if the transaction is finalized except the first enrolled record.
      This means that de-duplicated records in the transaction won't refer
      latest state (e.g. won't happen rolling back record state).
      
      The second one problem, the enrolled order is not always the same as the
      order in which the actions actually happened, the first enrolled record
      may succeed no actions (e.g. `destroy` has already succeeded on another
      record during `before_destroy`), it will lose to fire any transactional
      callbacks.
      
      To avoid both problems, we should attempt `committed!`/`rolledback!` to
      all enrolled records in the transaction.
      718a32ca
  9. 03 5月, 2019 1 次提交
    • Y
      Remove redundant `test_too_many_binds` · 36483cdb
      Yasuo Honda 提交于
      with `ActiveRecord::BindParameterTest#test_too_many_binds`
      
      sqlite adapter has its own `bind_params_length`, `ActiveRecord::BindParameterTest#test_too_many_binds` respects it.
      
      * Modified `ActiveRecord::BindParameterTest#test_too_many_binds` to show `bind_params_length` value
      
      ```
      $ git diff
      diff --git a/activerecord/test/cases/bind_parameter_test.rb b/activerecord/test/cases/bind_parameter_test.rb
      index 85685d1d00..83cd07f1d7 100644
      --- a/activerecord/test/cases/bind_parameter_test.rb
      +++ b/activerecord/test/cases/bind_parameter_test.rb
      @@ -108,6 +108,7 @@ def test_statement_cache_with_sql_string_literal
      
             def test_too_many_binds
               bind_params_length = @connection.send(:bind_params_length)
      +        p bind_params_length
      
               topics = Topic.where(id: (1 .. bind_params_length).to_a << 2**63)
               assert_equal Topic.count, topics.count
      $
      ```
      
      * Executed modified `ActiveRecord::BindParameterTest#test_too_many_binds`
      
      ```
      $ bin/test test/cases/bind_parameter_test.rb -n test_too_many_binds
      Using sqlite3
      Run options: -n test_too_many_binds --seed 47321
      
      999
      .
      
      Finished in 0.075249s, 13.2892 runs/s, 26.5784 assertions/s.
      1 runs, 2 assertions, 0 failures, 0 errors, 0 skips
      $
      ```
      36483cdb
  10. 02 5月, 2019 2 次提交
  11. 01 5月, 2019 1 次提交
    • Y
      Remove database specific sql statements from SQLCounter · 6030110f
      Yasuo Honda 提交于
      Every database executes different type of sql statement to get metadata then `ActiveRecord::TestCase` ignores these database specific sql statements to make `assert_queries` or `assert_no_queries` work consistently.
      
      Connection adapter already labels these statement by setting "SCHEMA" argument, this pull request makes use of "SCHEMA" argument to ignore metadata queries.
      
      Here are the details of these changes:
      
      * PostgresqlConnectionTest
      
      Each of PostgresqlConnectionTest modified just executes corresponding methods
      
      https://github.com/rails/rails/blob/fef174f5c524edacbcad846d68400e7fe114a15a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb#L182-L195
      
      ```ruby
              # Returns the current database encoding format.
              def encoding
                query_value("SELECT pg_encoding_to_char(encoding) FROM pg_database WHERE datname = current_database()", "SCHEMA")
              end
      
              # Returns the current database collation.
              def collation
                query_value("SELECT datcollate FROM pg_database WHERE datname = current_database()", "SCHEMA")
              end
      
              # Returns the current database ctype.
              def ctype
                query_value("SELECT datctype FROM pg_database WHERE datname = current_database()", "SCHEMA")
              end
      ```
      
      * BulkAlterTableMigrationsTest
      
      mysql2 adapter executes `SHOW KEYS FROM ...` to see if there is an index already created as below. I think the main concerns of these tests are how each database adapter creates or drops indexes then ignoring  `SHOW KEYS FROM` statement makes sense.
      
      https://github.com/rails/rails/blob/fef174f5c524edacbcad846d68400e7fe114a15a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb#L11
      
      ```ruby
                execute_and_free("SHOW KEYS FROM #{quote_table_name(table_name)}", "SCHEMA") do |result|
      ```
      
      * Temporary change not included in this commit to show which statements executed
      
      ```diff
      $ git diff
      diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb
      index 8e8ed494d9..df05f9bd16 100644
      --- a/activerecord/test/cases/migration_test.rb
      +++ b/activerecord/test/cases/migration_test.rb
      @@ -854,7 +854,7 @@ def test_adding_indexes
      
             classname = ActiveRecord::Base.connection.class.name[/[^:]*$/]
             expected_query_count = {
      -        "Mysql2Adapter"     => 3, # Adding an index fires a query every time to check if an index already exists or not
      +        "Mysql2Adapter"     => 1, # Adding an index fires a query every time to check if an index already exists or not
               "PostgreSQLAdapter" => 2,
             }.fetch(classname) {
               raise "need an expected query count for #{classname}"
      @@ -886,7 +886,7 @@ def test_removing_index
      
             classname = ActiveRecord::Base.connection.class.name[/[^:]*$/]
             expected_query_count = {
      -        "Mysql2Adapter"     => 3, # Adding an index fires a query every time to check if an index already exists or not
      +        "Mysql2Adapter"     => 1, # Adding an index fires a query every time to check if an index already exists or not
               "PostgreSQLAdapter" => 2,
             }.fetch(classname) {
               raise "need an expected query count for #{classname}"
      $
      ```
      
      * Executed these modified tests
      
      ```ruby
      $ ARCONN=mysql2 bin/test test/cases/migration_test.rb -n /index/
      Using mysql2
      Run options: -n /index/ --seed 8462
      
      F
      
      Failure:
      BulkAlterTableMigrationsTest#test_adding_indexes [/home/yahonda/git/rails/activerecord/test/cases/migration_test.rb:863]:
      3 instead of 1 queries were executed.
      Queries:
      SHOW KEYS FROM `delete_me`
      SHOW KEYS FROM `delete_me`
      ALTER TABLE `delete_me` ADD UNIQUE INDEX `awesome_username_index`  (`username`), ADD  INDEX `index_delete_me_on_name_and_age`  (`name`, `age`).
      Expected: 1
        Actual: 3
      
      bin/test test/cases/migration_test.rb:848
      
      F
      
      Failure:
      BulkAlterTableMigrationsTest#test_removing_index [/home/yahonda/git/rails/activerecord/test/cases/migration_test.rb:895]:
      3 instead of 1 queries were executed.
      Queries:
      SHOW KEYS FROM `delete_me`
      SHOW KEYS FROM `delete_me`
      ALTER TABLE `delete_me` DROP INDEX `index_delete_me_on_name`, ADD UNIQUE INDEX `new_name_index`  (`name`).
      Expected: 1
        Actual: 3
      
      bin/test test/cases/migration_test.rb:879
      
      ..
      
      Finished in 0.379245s, 10.5473 runs/s, 7.9105 assertions/s.
      4 runs, 3 assertions, 2 failures, 0 errors, 0 skips
      $
      ```
      
      * ActiveRecord::ConnectionAdapters::Savepoints
      
      Left `self.ignored_sql` to ignore savepoint related statements because these SQL statements are not related "SCHEMA"
      
      ```
      self.ignored_sql = [/^SAVEPOINT/, /^ROLLBACK TO SAVEPOINT/, /^RELEASE SAVEPOINT/]
      ```
      
      https://github.com/rails/rails/blob/fef174f5c524edacbcad846d68400e7fe114a15a/activerecord/lib/active_record/connection_adapters/abstract/savepoints.rb#L10-L20
      
      ```ruby
            def create_savepoint(name = current_savepoint_name)
              execute("SAVEPOINT #{name}")
            end
      
            def exec_rollback_to_savepoint(name = current_savepoint_name)
              execute("ROLLBACK TO SAVEPOINT #{name}")
            end
      
            def release_savepoint(name = current_savepoint_name)
              execute("RELEASE SAVEPOINT #{name}")
            end
      ```
      6030110f
  12. 30 4月, 2019 2 次提交
  13. 29 4月, 2019 1 次提交
  14. 27 4月, 2019 1 次提交
    • R
      Fix merging left_joins to maintain its own `join_type` context · 20ede2e2
      Ryuta Kamizono 提交于
      This fixes a regression for #35864.
      
      Usually, stashed joins (mainly eager loading) are performed as LEFT
      JOINs.
      But the case of merging joins/left_joins of different class, that
      (stashed) joins are performed as the same `join_type` as the parent
      context for now.
      Since #35864, both (joins/left_joins) stashed joins might be contained
      in `joins_values`, so each stashed joins should maintain its own
      `join_type` context.
      
      Fixes #36103.
      20ede2e2
  15. 25 4月, 2019 2 次提交
  16. 24 4月, 2019 8 次提交
  17. 22 4月, 2019 5 次提交
    • R
      5454cc40
    • R
      70e255b9
    • R
      Remove useless `set_value` / `get_value` helper methods · 89b86640
      Ryuta Kamizono 提交于
      Those helper methods makes relation values access 15% slower.
      
      https://gist.github.com/kamipo/e64439f7a206e1c5b5c69d92d982828e
      
      Before (02b5b8cb):
      
      ```
      Warming up --------------------------------------
              #limit_value   237.074k i/100ms
          #limit_value = 1   222.052k i/100ms
      Calculating -------------------------------------
              #limit_value      6.477M (± 2.9%) i/s -     32.479M in   5.019475s
          #limit_value = 1      5.297M (± 4.3%) i/s -     26.424M in   4.999933s
      ```
      
      After (this change):
      
      ```
      Warming up --------------------------------------
              #limit_value   261.109k i/100ms
          #limit_value = 1   239.646k i/100ms
      Calculating -------------------------------------
              #limit_value      7.412M (± 1.6%) i/s -     37.077M in   5.003345s
          #limit_value = 1      6.134M (± 1.0%) i/s -     30.675M in   5.000908s
      ```
      89b86640
    • R
      PERF: 20% faster pk attribute access · b6828fc9
      Ryuta Kamizono 提交于
      I've realized that `user.id` is 20% slower than `user.name` in the
      benchmark (https://github.com/rails/rails/pull/35987#issuecomment-483882480).
      
      The reason that performance difference is that `self.class.primary_key`
      method call is a bit slow.
      
      Avoiding that method call will make almost attribute access faster and
      `user.id` will be completely the same performance with `user.name`.
      
      Before (02b5b8cb):
      
      ```
      Warming up --------------------------------------
                   user.id   140.535k i/100ms
                user['id']    96.549k i/100ms
                 user.name   158.110k i/100ms
              user['name']    94.507k i/100ms
             user.changed?    19.003k i/100ms
       user.saved_changes?    25.404k i/100ms
      Calculating -------------------------------------
                   user.id      2.231M (± 0.9%) i/s -     11.243M in   5.040066s
                user['id']      1.310M (± 1.3%) i/s -      6.565M in   5.012607s
                 user.name      2.683M (± 1.2%) i/s -     13.439M in   5.009392s
              user['name']      1.322M (± 0.9%) i/s -      6.615M in   5.003239s
             user.changed?    201.999k (±10.9%) i/s -      1.007M in   5.091195s
       user.saved_changes?    258.214k (±17.1%) i/s -      1.245M in   5.007421s
      ```
      
      After (this change):
      
      ```
      Warming up --------------------------------------
                   user.id   158.364k i/100ms
                user['id']   106.412k i/100ms
                 user.name   158.644k i/100ms
              user['name']   107.518k i/100ms
             user.changed?    19.082k i/100ms
       user.saved_changes?    24.886k i/100ms
      Calculating -------------------------------------
                   user.id      2.768M (± 1.1%) i/s -     13.936M in   5.034957s
                user['id']      1.507M (± 2.1%) i/s -      7.555M in   5.017211s
                 user.name      2.727M (± 1.5%) i/s -     13.643M in   5.004766s
              user['name']      1.521M (± 1.3%) i/s -      7.634M in   5.018321s
             user.changed?    200.865k (±11.1%) i/s -    992.264k in   5.044868s
       user.saved_changes?    269.652k (±10.5%) i/s -      1.344M in   5.077972s
      ```
      b6828fc9
    • R
      Remove never used `database_selector` class accessor · 02b5b8cb
      Ryuta Kamizono 提交于
      It was never used from the beginning.
      02b5b8cb
  18. 21 4月, 2019 1 次提交
    • R
      Avoid method call if `@transaction_state` is not finalized · f9326e56
      Ryuta Kamizono 提交于
      Method call in Ruby is a bit slow.
      
      This makes attribute access 10% faster by avoiding method call
      (`sync_with_transaction_state`).
      
      Before (96cf7e0e):
      
      ```
      Warming up --------------------------------------
                   user.id   131.291k i/100ms
                user['id']    91.786k i/100ms
                 user.name   151.605k i/100ms
              user['name']    92.664k i/100ms
             user.changed?    17.772k i/100ms
       user.saved_changes?    23.909k i/100ms
      Calculating -------------------------------------
                   user.id      1.988M (± 7.0%) i/s -      9.978M in   5.051474s
                user['id']      1.155M (± 5.8%) i/s -      5.783M in   5.022672s
                 user.name      2.450M (± 4.3%) i/s -     12.280M in   5.021234s
              user['name']      1.263M (± 2.1%) i/s -      6.394M in   5.066638s
             user.changed?    175.070k (±13.3%) i/s -    853.056k in   5.011555s
       user.saved_changes?    259.114k (±11.8%) i/s -      1.267M in   5.001260s
      ```
      
      After (this change):
      
      ```
      Warming up --------------------------------------
                   user.id   137.625k i/100ms
                user['id']    96.054k i/100ms
                 user.name   156.379k i/100ms
              user['name']    94.795k i/100ms
             user.changed?    18.172k i/100ms
       user.saved_changes?    24.337k i/100ms
      Calculating -------------------------------------
                   user.id      2.201M (± 0.5%) i/s -     11.010M in   5.002955s
                user['id']      1.320M (± 1.0%) i/s -      6.628M in   5.021293s
                 user.name      2.677M (± 1.6%) i/s -     13.449M in   5.024399s
              user['name']      1.314M (± 1.8%) i/s -      6.636M in   5.051444s
             user.changed?    190.588k (±11.1%) i/s -    944.944k in   5.065848s
       user.saved_changes?    262.782k (±12.1%) i/s -      1.290M in   5.028080s
      ```
      f9326e56
  19. 20 4月, 2019 2 次提交