1. 01 2月, 2015 3 次提交
    • S
      Attribute assignment and type casting has nothing to do with columns · 70ac0729
      Sean Griffin 提交于
      It's finally finished!!!!!!! The reason the Attributes API was kept
      private in 4.2 was due to some publicly visible implementation details.
      It was previously implemented by overloading `columns` and
      `columns_hash`, to make them return column objects which were modified
      with the attribute information.
      
      This meant that those methods LIED! We didn't change the database
      schema. We changed the attribute information on the class. That is
      wrong! It should be the other way around, where schema loading just
      calls the attributes API for you. And now it does!
      
      Yes, this means that there is nothing that happens in automatic schema
      loading that you couldn't manually do yourself. (There's still some
      funky cases where we hit the connection adapter that I need to handle,
      before we can turn off automatic schema detection entirely.)
      
      There were a few weird test failures caused by this that had to be
      fixed. The main source came from the fact that the attribute methods are
      now defined in terms of `attribute_names`, which has a clause like
      `return [] unless table_exists?`. I don't *think* this is an issue,
      since the only place this caused failures were in a fake adapter which
      didn't override `table_exists?`.
      
      Additionally, there were a few cases where tests were failing because a
      migration was run, but the model was not reloaded. I'm not sure why
      these started failing from this change, I might need to clear an
      additional cache in `reload_schema_from_cache`. Again, since this is not
      normal usage, and it's expected that `reset_column_information` will be
      called after the table is modified, I don't think it's a problem.
      
      Still, test failures that were unrelated to the change are worrying, and
      I need to dig into them further.
      
      Finally, I spent a lot of time debugging issues with the mutex used in
      `define_attribute_methods`. I think we can just remove that method
      entirely, and define the attribute methods *manually* in the call to
      `define_attribute`, which would simplify the code *tremendously*.
      
      Ok. now to make this damn thing public, and work on moving it up to
      Active Model.
      70ac0729
    • H
      changed deleted_tables list to set · e773ca99
      Hyonjee Joo 提交于
      e773ca99
    • S
      Remove `AttributeSet#initialized_keys` · aebba01f
      Sean Griffin 提交于
      This method doesn't need to be lazy, as it is never called from reads.
      The only time it is called are in write cases, where we're about to loop
      through the results of it, and build the attribute objects anyway. So we
      don't gain anything by dodging the instantiation here. This is the only
      method that coupled `AttributeSet` to `LazyAttributeHash`, so removing
      it puts us back in a place where we can use a normal hash instead.
      aebba01f
  2. 31 1月, 2015 2 次提交
    • S
      Remove most type related predicates from `Column` · b93b39ef
      Sean Griffin 提交于
      Remaining are `limit`, `precision`, `scale`, and `type` (the symbol
      version). These will remain on the column, since they mirror the options
      to the `column` method in the schema definition DSL
      b93b39ef
    • S
      Remove most uses of `Column#cast_type` · 155b1b7f
      Sean Griffin 提交于
      The goal is to remove the type object from the column, and remove
      columns from the type casting process entirely. The primary motivation
      for this is clarity. The connection adapter does not have sufficient
      type information, since the type we want to work with might have been
      overriden at the class level. By taking this object from the column,
      it is easy to mistakenly think that the column object which exists on
      the connection adapter is sufficient. It isn't.
      
      A concrete example of this is `serialize`. In 4.2 and earlier, `where`
      worked in a very inconsistent and confusing manner. If you passed a
      single value to `where`, it would serialize it before querying, and do
      the right thing. However, passing it as part of an array, hash, or range
      would cause it to not work. This is because it would stop using prepared
      statements, so the type casting would come from arel. Arel would have no
      choice but to get the column from the connection adapter, which would
      treat it as any other string column, and query for the wrong value.
      
      There are a handful of cases where using the column object to find the
      cast type is appropriate. These are cases where there is not actually a
      class involved, such as the migration DSL, or fixtures. For all other
      cases, the API should be designed as such that the type is provided
      before we get to the connection adapter. (For an example of this, see
      the work done to decorate the arel table object with a type caster, or
      the introduction of `QueryAttribute` to `Relation`).
      
      There are times that it is appropriate to use information from the
      column to change behavior in the connection adapter. These cases are
      when the primitive used to represent that type before it goes to the
      database does not sufficiently express what needs to happen. An example
      of this that affects every adapter is binary vs varchar, where the
      primitive used for both is a string. In this case it is appropriate to
      look at the column object to determine which quoting method to use, as
      this is something schema dependent.
      
      An example of something which would not be appropriate is to look at the
      type and see that it is a datetime, and performing string parsing when
      given a string instead of a date.  This is the type of logic that should
      live entirely on the type. The value which comes out of the type should
      be a sufficiently generic primitive that the adapter can be expected to
      know how to work with it.
      
      The one place that is still using the column for type information which
      should not be necessary is the connection adapter type caster which is
      sometimes given to the arel table when we can't find the associated
      table. This will hopefully go away in the near future.
      155b1b7f
  3. 30 1月, 2015 2 次提交
  4. 29 1月, 2015 6 次提交
  5. 28 1月, 2015 11 次提交
    • H
      Provide a better error message on :required association · 9a6c6c6f
      Henrik Nygren 提交于
      Fixes #18696.
      9a6c6c6f
    • S
      Remove Relation#bind_params · b06f64c3
      Sean Griffin 提交于
      `bound_attributes` is now used universally across the board, removing
      the need for the conversion layer. These changes are mostly mechanical,
      with the exception of the log subscriber. Additional, we had to
      implement `hash` on the attribute objects, so they could be used as a
      key for query caching.
      b06f64c3
    • F
      fix typo still cause -> still causes · 31dd1ca5
      Felix Schäfer 提交于
      31dd1ca5
    • S
      3a551b97
    • S
      Use an `Attribute` object to represent a bind value · 6c235dd9
      Sean Griffin 提交于
      The column is primarily used for type casting, which we're trying to
      separate from the idea of a column. Since what we really need is the
      combination of a name, type, and value, let's use the object that we
      already have to represent that concept, rather than this tuple. No
      consumers of the bind values have been changed, only the producers
      (outside of tests which care too much about internals). This is
      *finally* possible since the bind values are now produced from a
      reasonable number of lcoations.
      6c235dd9
    • S
      Don't rely on the internal representation of join values · 102a5272
      Sean Griffin 提交于
      I'm going to be extracting this logic into a clause class, things need
      to go through a method and not access the values hash directly.
      102a5272
    • S
      Minor refactorings on `Relation#build_joins` · ae299dd4
      Sean Griffin 提交于
      Attempting to grok this code by refactoring it as I go through it.
      ae299dd4
    • S
      `WhereClause#predicates` does not need to be public · d26dd008
      Sean Griffin 提交于
      The only place it was accessed was in tests. Many of them have another
      way that they can test their behavior, that doesn't involve reaching
      into internals as far as they did. `AssociationScopeTest` is testing a
      situation where the where clause would have one bind param per
      predicate, so it can just ignore the predicates entirely. The where
      chain test was primarly duplicating the logic tested on `WhereClause`
      directly, so I instead just make sure it calls the appropriate method
      which is fully tested in isolation.
      d26dd008
    • S
      Use the `WhereClause` ast building logic for having · c2c95cd2
      Sean Griffin 提交于
      c2c95cd2
    • S
      Move where grouping into `WhereClause` · a5fcdae0
      Sean Griffin 提交于
      a5fcdae0
    • S
      Unify access to bind values on Relation · 16ce2eec
      Sean Griffin 提交于
      The bind values can come from four places. `having`, `where`, `joins`,
      and `from` when selecting from a subquery that contains binds. These
      need to be kept in a specific order, since the clauses will always
      appear in that order. Up until recently, they were not.
      
      Additionally, `joins` actually did keep its bind values in a separate
      location (presumably because it's the only case that people noticed was
      broken). However, this meant that anything accessing just `bind_values`
      was broken (which most places were). This is no longer possible, there
      is only a single way to access the bind values, and it includes joins in
      the proper location. The setter was removed yesterday, so breaking `+=`
      cases is not possible.
      
      I'm still not happy that `joins` is putting it's bind values on the
      Arel AST, and I'm planning on refactoring it further, but this removes a
      ton of bug cases.
      16ce2eec
  6. 27 1月, 2015 11 次提交
    • R
      Restore useful documentation removed at · 1bc30b18
      Rafael Mendonça França 提交于
      3729103e
      
      [ci skip]
      1bc30b18
    • T
      Update model_schema.rb [ci skip] · 3729103e
      Takehiro Adachi 提交于
      Overriding these methods may cause unexpected results since
      "table_name=" does more then just setting the "@table_name".
      
      ref: https://github.com/rails/rails/pull/18622#issuecomment-70874358
      3729103e
    • S
      Move the `from` bind logic to a `FromClause` class · bdc51416
      Sean Griffin 提交于
      Contrary to my previous commit message, it wasn't overkill, and led to
      much cleaner code.
      
      [Sean Griffin & anthonynavarre]
      bdc51416
    • S
      Remove `Relation#bind_values=` · 8436e2c2
      Sean Griffin 提交于
      The last place that was assigning it was when `from` is called with a
      relation to use as a subquery. The implementation was actually
      completely broken, and would break if you called `from` more than once,
      or if you called it on a relation, which also had its own join clause,
      as the bind values would get completely scrambled. The simplest solution
      was to just move it into its own array, since creating a `FromClause`
      class for this would be overkill.
      8436e2c2
    • S
      765a3123
    • S
      Remove `Relation#build_where` · 76661dc6
      Sean Griffin 提交于
      All of its uses have been moved to better places
      76661dc6
    • S
      Go through normal `where` logic in `AssociationScope` · 6a7ac40d
      Sean Griffin 提交于
      This removes the need to duplicate much of the logic in `WhereClause`
      and `PredicateBuilder`, simplifies the code, removes the need for the
      connection adapter to be continuously passed around, and removes one
      place that cares about the internal representation of `bind_values`
      
      Part of the larger refactoring to change how binds are represented
      internally
      
      [Sean Griffin & anthonynavarre]
      6a7ac40d
    • S
      Ensure the type caster object given to Arel is always marshallable · 9d4d2e7f
      Sean Griffin 提交于
      The Relation will ultimately end up holding a reference to the arel
      table object, and its associated type caster. If this is a
      `TypeCaster::Connection`, that means it'll hold a reference to the
      connection adapter, which cannot be marshalled. We can work around this
      by just holding onto the class object instead. It's ugly, but I'm hoping
      to remove the need for the connection adapter type caster in the future
      anyway.
      
      [Sean Griffin & anthonynavarre]
      9d4d2e7f
    • S
      Change `having_values` to use the `WhereClause` class · 39f2c3b3
      Sean Griffin 提交于
      This fixed an issue where `having` can only be called after the last
      call to `where`, because it messes with the same `bind_values` array.
      With this change, the two can be called as many times as needed, in any
      order, and the final query will be correct. However, once something
      assigns `bind_values`, that stops. This is because we have to move all
      of the bind values from the having clause over to the where clause since
      we can't differentiate the two, and assignment was likely in the form
      of:
      
      `relation.bind_values += other.bind_values`
      
      This will go away once we remove all places that are assigning
      `bind_values`, which is next on the list.
      
      While this fixes a bug that was present in at least 4.2 (more likely
      present going back as far as 3.0, becoming more likely in 4.1 and later
      as we switched to prepared statements in more cases), I don't think this
      can be easily backported. The internal changes to `Relation` are
      non-trivial, anything that involves modifying the `bind_values` array
      would need to change, and I'm not confident that we have sufficient test
      coverage of all of those locations (when `having` was called with a hash
      that could generate bind values).
      
      [Sean Griffin & anthonynavarre]
      39f2c3b3
    • S
      Improve consistency of counter caches updating in memory · 1152219f
      Sean Griffin 提交于
      When we made sure that the counter gets updated in memory, we only did
      it on the has many side. The has many side only does the update if the
      belongs to cannot. The belongs to side was updated to update the counter
      cache (if it is able). This means that we need to check if the
      belongs_to is able to update in memory on the has_many side.
      
      We also found an inconsistency where the reflection names were used to
      grab the association which should update the counter cache. Since
      reflection names are now strings, this means it was using a different
      instance than the one which would have the inverse instance set.
      
      Fixes #18689
      
      [Sean Griffin & anthonynavarre]
      1152219f
    • S
      Move flattening records added to an association farther out · 025187d9
      Sean Griffin 提交于
      There are many ways that things end up getting passed to `concat`. Not
      all of those entry points called `flatten` on their input. It seems that
      just about every method that is meant to take a single record, or that
      splats its input, is meant to also take an array. `concat` is the
      earliest point that is common to all of the methods which add records to
      the association. Partially fixes #18689
      025187d9
  7. 26 1月, 2015 5 次提交