1. 24 8月, 2020 5 次提交
  2. 23 8月, 2020 1 次提交
    • R
      Support storing demodulized class name for polymorphic type · 62cfbdf3
      Ryuta Kamizono 提交于
      This is an alternative of #29722.
      
      Before Rails 6.1, storing demodulized class name is supported only for
      STI type by `store_full_sti_class` class attribute.
      
      Now `store_full_class_name` class attribute can handle both STI and
      polymorphic types.
      
      Closes #29722.
      
      See also #29601, #32048, #32148.
      62cfbdf3
  3. 19 8月, 2020 1 次提交
  4. 17 8月, 2020 1 次提交
    • R
      Fix eager loading that non-select columns will be loaded · 46393182
      Ryuta Kamizono 提交于
      Related to #35210.
      
      We sometimes use `select` to limit unused columns for performance.
      
      For example, `GET /posts/1` (post detail) usually use (almost) all
      columns, but `GET /posts` (post list) does not always use all columns
      (e.g. use `id` and `title` for the list view, but `body` is not used).
      
      If an association is eager loaded, the limited `select` doesn't works as
      expected, eager loading will load all columns on the model, plus also
      load the `select` columns additionally. It works differently with
      natural load and preload. It means that changing natural load or preload
      to eager load (or vice versa) is unsafe.
      
      This fixes eager loading that always load all columns (plus extra
      `select` columns), to respect the `select` columns like as others.
      
      ```ruby
      post = Post.select("UPPER(title) AS title").first
      post.title # => "WELCOME TO THE WEBLOG"
      post.body  # => ActiveModel::MissingAttributeError
      
      # Rails 6.0 (ignore the `select` values)
      post = Post.select("UPPER(title) AS title").eager_load(:comments).first
      post.title # => "Welcome to the weblog"
      post.body  # => "Such a lovely day"
      
      # Rails 6.1 (respect the `select` values)
      post = Post.select("UPPER(title) AS title").eager_load(:comments).first
      post.title # => "WELCOME TO THE WEBLOG"
      post.body  # => ActiveModel::MissingAttributeError
      ```
      46393182
  5. 16 8月, 2020 1 次提交
    • R
      Fix preloader to associate preloaded records by default · f80179db
      Ryuta Kamizono 提交于
      Someone had relied on the behavior that preloading with a given scope,
      but the behavior has lost in #35496 to fix the minor bug that unloading
      through association.
      
      Basically we don't guarantee the internal behavior, but the bugfix can
      be achieved without any breaking change, so I've restored the lost
      functionality.
      
      Fixes #36638.
      Fixes #37720.
      f80179db
  6. 15 8月, 2020 1 次提交
  7. 14 8月, 2020 4 次提交
    • E
      Fix incorrect removal of current_shard in establish_connection · ba2f38e4
      eileencodes 提交于
      If we enter a `connected_to` block and call `establish_connection` like
      the test added here we need to ensure that `shard: current_shard` is passed
      to the handler, otherwise the connection will be established on
      `default` not on `shard_one`.
      Co-authored-by: NJohn Crepezzi <john.crepezzi@gmail.com>
      ba2f38e4
    • D
      Rename single letter variables · d79519f2
      Daniel Colson 提交于
      After renaming `Man` to `Human` the variable letter `m` in these tests
      ends up being pretty confusing. Rather than rename it to `h`, this
      commit replaces it with the full word `human`.
      
      Since I was already renaming things, I also went ahead and replaced `f`
      with `face`, `i` with `interest`, and `a` with `author`.
      d79519f2
    • D
      Replace test `Man` with `Human` · 7f938cac
      Daniel Colson 提交于
      The commit replaces the `Man` model used in tests with a `Human` model. It
      also replaces the existing `Human` model with a `SuperHuman` model
      inheriting from `Human`.
      
      While this may seem like a cosmetic change, I see it as more of an
      inclusivity change. I think it makes sense for a number of reasons:
      
      * Prior to this commit the `Human` model inherited from `Man`. At best
        this makes no sense (it should be the other way around). At worst it
        is offensive and harmful to the community.
      * It doesn't seem inclusive to me to have exclusively male-gendered
        examples in the codebase.
      * There is no particular reason for these examples to be gendered.
      * `man` is hard to grep for, since it also matches `many, manager,
        manual, etc`
      
      For the most part this is a simple search and replace. The one exception
      to that is that I had to add the table name to the model so we could use
      "humans" instead of "humen".
      7f938cac
    • R
      Fix incorrect result when eager loading with duplicated through association with join scope Part 2 · 62de28fb
      Ryuta Kamizono 提交于
      Follow up of #40000.
      
      In #40000, `eager_load(:general_categorizations, :general_posts)` works,
      but `eager_load(:general_posts, :general_categorizations)` doesn't work
      yet.
      
      This implements the deduplication for the case of reversed eager loading
      order.
      62de28fb
  8. 11 8月, 2020 1 次提交
    • E
      Fix missed establish_connection · 919eb6dc
      eileencodes 提交于
      In `connected_to` one of the deprecated arguments wasn't well tested so
      the incorrect methods signature wasn't caught by the tests.
      
      This change updates the caller when `connected_to` uses the database
      key.
      
      I've also cleaned up a few arguments that weren't necessary. Since
      the handler methods set defaults for the `shard` key, we don't need to
      pass that in `establish_connection` when not using the sharding API.
      919eb6dc
  9. 10 8月, 2020 1 次提交
    • E
      Add helper method for resetting connection handlers in tests · 61ab1543
      eileencodes 提交于
      This change makes a helper method for resetting connection handlers in
      the Active Record tests. The change here is relatively small and may
      seem unnecessary. The reason we're pushing this change is for upcoming
      refactoring to connection management. This change will mean that we can
      update one location instead of 9+ files to reset connections. It will
      reduce the diff of our refactoring and make reusing this code easier in
      the future.
      
      The method name chosen is purposefully `clean_up_connection_handler`
      over `clean_up_connection_handlers` because in the future there will
      only be one handler.
      Co-authored-by: NJohn Crepezzi <john.crepezzi@gmail.com>
      61ab1543
  10. 08 8月, 2020 3 次提交
    • E
      Update connection methods to use kwargs · d3061cda
      eileencodes 提交于
      This change ensures that the connection methods are using kwargs instead
      of positional arguments. This change may look unnecessary but we're
      working on refactoring connection management to make it more robust and
      flexible so the method signatures of the methods changed here will
      continue to evolve and change.
      
      This commit does not change any released public APIs. The `shard` and
      `owner_name` arguments were added in 6.1 which is not released yet.
      Using kwargs will allow these methods to be more flexible and not get
      super ugly as we change their underlying behavior. The kwargs let us
      support multiple non-positional arguments with default.
      Co-authored-by: NJohn Crepezzi <john.crepezzi@gmail.com>
      d3061cda
    • E
      Rename `pool_key` to `shard` · 85ef2196
      eileencodes 提交于
      This change renames the following:
      
      * `current_pool_key` -> `current_shard`
      * `default_pool_key` -> `default_shard`
      * `pool_key` -> `shard`
      
      Originally we had intended to name the `shard` as `pool_key` because
      when we implemented the internal private API we weren't sure how it was
      going to be used for sharding and wanted to implement behavior without
      promising a public API. Now that we have a public API for sharding it's
      better to use the same name everywhere rather than have one name for
      private APIs and one name for public APIs. This should make
      contributions and tracking down bugs easier in the future.
      
      This PR doesn't require any deprecations because the sharding API is
      unreleased and so is all the internal code that was using `pool_key`.
      Co-authored-by: NJohn Crepezzi <john.crepezzi@gmail.com>
      85ef2196
    • E
      Remove unnecessary with_temporary_connection_pool calls · 9289232b
      eileencodes 提交于
      While debugging a different problem I'm working on I realized that this
      method `with_temporary_connection_pool` isn't necessary in most of the
      cases we're using it for.
      
      Anywhere we establish new connections inside the block won't throw away
      those new connections. I also removed this from places that can use the
      existing connection and don't need a new temporary pool. I'm not sure if
      this file was using it in many places because of copy / paste or real
      issues that are no longer present.
      9289232b
  11. 07 8月, 2020 3 次提交
    • R
      Fix incorrect result when eager loading with duplicated through association with join scope · 10b36e81
      Ryuta Kamizono 提交于
      I had found the issue while working on fixing #33525.
      
      That is if duplicated association has a scope which has `where` with
      explicit table name condition (e.g. `where("categories.name": "General")`),
      that condition in all duplicated associations will filter the first one
      only, other all duplicated associations are not filtered, since
      duplicated joins will be aliased except the first one (e.g.
      `INNER JOIN "categories" "categories_categorizations"`).
      
      ```ruby
      class Author < ActiveRecord::Base
        has_many :general_categorizations, -> { joins(:category).where("categories.name": "General") }, class_name: "Categorization"
        has_many :general_posts, through: :general_categorizations, source: :post
      end
      
      authors = Author.eager_load(:general_categorizations, :general_posts).to_a
      ```
      
      Generated eager loading query:
      
      ```sql
      SELECT "authors"."id" AS t0_r0, ... FROM "authors"
      
      -- `has_many :general_categorizations, -> { joins(:category).where("categories.name": "General") }`
      LEFT OUTER JOIN "categorizations" ON "categorizations"."author_id" = "authors"."id"
      INNER JOIN "categories" ON "categories"."id" = "categorizations"."category_id" AND "categories"."name" = ?
      
      -- `has_many :general_posts, through: :general_categorizations, source: :post`
      ---- duplicated `through: :general_categorizations` part
      LEFT OUTER JOIN "categorizations" "general_categorizations_authors_join" ON "general_categorizations_authors_join"."author_id" = "authors"."id"
      INNER JOIN "categories" "categories_categorizations" ON "categories_categorizations"."id" = "general_categorizations_authors_join"."category_id" AND "categories"."name" = ? -- <-- filtering `"categories"."name" = ?` won't work
      ---- `source: :post` part
      LEFT OUTER JOIN "posts" ON "posts"."id" = "general_categorizations_authors_join"."post_id"
      ```
      
      Originally eager loading with join scope didn't work before Rails 5.2
      (#29413), and duplicated through association with join scope raised a
      duplicated alias error before alias tracking is improved in 590b045e.
      
      But now it will potentially be got incorrect result instead of an error,
      it is worse than an error.
      
      To fix the issue, it makes eager loading to deduplicate / re-use
      duplicated through association if possible, like as `preload`.
      
      ```sql
      SELECT "authors"."id" AS t0_r0, ... FROM "authors"
      
      -- `has_many :general_categorizations, -> { joins(:category).where("categories.name": "General") }`
      LEFT OUTER JOIN "categorizations" ON "categorizations"."author_id" = "authors"."id"
      INNER JOIN "categories" ON "categories"."id" = "categorizations"."category_id" AND "categories"."name" = ?
      
      -- `has_many :general_posts, through: :general_categorizations, source: :post`
      ---- `through: :general_categorizations` part is deduplicated / re-used
      LEFT OUTER JOIN "posts" ON "posts"."id" = "categorizations"."post_id"
      ```
      
      Fixes #32819.
      10b36e81
    • R
      Fix deserializing enum mapping nil · afeb7568
      Ryuta Kamizono 提交于
      Follow up to #38086.
      
      User assigned nil is type casted by #38086, but loaded nil from database
      does not yet, this fixes that.
      afeb7568
    • R
      Don't use arel factory methods for creating join nodes · e8cf45dc
      Ryuta Kamizono 提交于
      Directly `klass.new` is clear enough than factory methods.
      e8cf45dc
  12. 05 8月, 2020 1 次提交
  13. 04 8月, 2020 2 次提交
    • G
      Move advisory locks to own connection handler. · 45add344
      Guo Xiang Tan 提交于
      Removes the use of `ActiveRecord::AdvisoryLockBase` since it inherits
      from `ActiveRecord::Base` and hence share module attributes that are defined in `ActiveRecord::Base`.
      This is problematic because establishing connections through
      `ActiveRecord::AdvisoryLockBase` can end up changing state of the default
      connection handler of `ActiveRecord::Base` leading to unexpected
      behaviors in a Rails application.
      
      In the case of https://github.com/rails/rails/issues/39157,
      
      Running migrations with `rails db:migrate:primary_shard_one` was not working as
      the application itself defined the following
      
      ```
      class ApplicationRecord < ActiveRecord::Base
        self.abstract_class = true
      
        connects_to shards: {
          default: { writing: :primary },
          shard_one: { writing: :primary_shard_one }
        }
      end
      ```
      
      In the database migrate rake task, the default connection was
      established with the database config of `primary_shard_one`. However,
      the default connection was altered to that of `primary` because
      `ActiveRecord::AdvisoryLockBase.establish_connection` ended up loading
      `ApplicationRecord` which calls `connects_to shards:`. Since all we
      really need here is just a normal database connection, we can avoid
      accidentally altering the default connection handler state during the migration
      by creating a custom connection handler used for retrieving a connection.
      45add344
    • F
  14. 03 8月, 2020 1 次提交
    • A
      The abstract parent class file generated via generator should not be pluralized · b9879bb8
      Abhay Nikam 提交于
      Currently, the file generated via the generator is pluralized
      but the parent class is singluar.
      
      example: bundle exec rails g scaffold Pet name:string --database=animals
      
      The above command should generate: apps/models/animals_record.rb
      
      but the pets model would inherit from: `AnimalRecord` as
      `"animals".classify` would be `Animal`
      
      This will throw the `uninitialized constant AnimalRecord Did you mean? AnimalsRecord`
      error.
      b9879bb8
  15. 01 8月, 2020 1 次提交
    • R
      Avoid implicit `create_with` for `StiClass.all` · bdaf273c
      Ryuta Kamizono 提交于
      Regardless of whether doing implicit `create_with` or not, `sti_name` is
      set by `ensure_proper_type`.
      
      https://github.com/rails/rails/blob/11f54e12b992f6c8d29fd9bedd89ac438a928a2f/activerecord/lib/active_record/inheritance.rb#L289-L304
      
      The purpose of the `create_with` is to suppress/override `type_condition`,
      since `type_condition` will be an array of all sti subclass names for
      `scope_for_create`, it is not desired behavior for `scope_for_create`,
      and it will cause `find_sti_class` to fail.
      
      That undesired behavior was derived from `In` arel node was accidentally
      a subclass of `Equality` node, IMHO it is considered as almost a bug.
      But someone depends on the behavior for now (see also #39288).
      
      Unfortunately that implicit `create_with` had an unwanted side effect to
      fail `or` with STI subclass relation #39956.
      
      This changes a way to suppress/override `type_condition`, from doing
      implicit `create_with` to excepting `type_condition` in `scope_for_create`,
      to fix the `or` issue #39956.
      bdaf273c
  16. 31 7月, 2020 1 次提交
    • A
      Adds db:migrate:redo:NAME support for multidbs · a636aa94
      André Luis Leal Cardoso Junior 提交于
      On current master with multiple DBs configured, calling db:migrate:redo fails when trying to run db:rollback.
      
      Before:
      
      ```
      » bin/rails db:migrate:redo
      rake aborted!
      You're using a multiple database application. To use `db:rollback` you must run the namespaced task with a VERSION. Available tasks are db:rollback:primary and db:rollback:secondary.
      Tasks: TOP => db:rollback
      (See full trace by running task with --trace)
      ```
      
      After:
      
      ```
      » bin/rails db:migrate:redo
      rake aborted!
      You're using a multiple database application. To use `db:migrate:redo` you must run the namespaced task with a VERSION. Available tasks are db:migrate:redo:primary and db:migrate:redo:secondary.
      Tasks: TOP => db:migrate:redo
      (See full trace by running task with --trace)
      ```
      
      Running the namespaced version:
      
      ```
      » bin/rails db:migrate:redo:secondary
      == 20200728162820 CreateAnimals: reverting ====================================
      -- drop_table(:animals)
         -> 0.0025s
      == 20200728162820 CreateAnimals: reverted (0.0047s) ===========================
      
      == 20200728162820 CreateAnimals: migrating ====================================
      -- create_table(:animals)
         -> 0.0028s
      == 20200728162820 CreateAnimals: migrated (0.0029s) ===========================
      ```
      a636aa94
  17. 30 7月, 2020 2 次提交
    • G
      96a1c88e
    • E
      Generate abstract class when generating scaffold in another database · 261cbcd2
      eileencodes 提交于
      This PR ensures that when you're generating a scaffold or model and that
      model should belong to another database it will create an abstract class
      if it doesn't already exist.
      
      The new abstract class will ensure that the new model inherits from that
      class, but will not be deleted if the scaffold is deleted. This is
      because Rails can't know if you have other models inheriting from that
      class so we don't want to revoke that if the scaffold is destroyed.
      
      If the abstract class already exists it won't be created twice. If the
      options for `parent` are set, the generator will use that as the
      abstract class instead of creating one. The generated abstract class
      will add the writing connection automatically, users need to add the
      reading connection themselves as Rails doesn't know which is the reading
      connection.
      Co-authored-by: NJohn Crepezzi <john.crepezzi@gmail.com>
      261cbcd2
  18. 28 7月, 2020 2 次提交
    • R
      a -> an [ci skip] · c6f02cbb
      Ryuta Kamizono 提交于
      c6f02cbb
    • R
      Move `column_for_attribute` into `ModelSchema` · 47aee7ff
      Ryuta Kamizono 提交于
      `type_for_attribute` was added (extracted from `column_for_attribute`)
      in `ModelSchema` at c93dbfef, I sometimes had expected that both
      `column_for_attribute` and `type_for_attribute` exists in `ModelSchema`,
      but `column_for_attribute` had existed in `AttributeMethods` even though
      it's not very relevant to attribute methods (getting column object is no
      longer useful for most end users since type object is separated from
      column object, by the way).
      
      So I've moved `column_for_attribute` to the same place with
      `type_for_attribute` to less confusion a bit.
      47aee7ff
  19. 27 7月, 2020 3 次提交
  20. 25 7月, 2020 3 次提交
  21. 23 7月, 2020 2 次提交