| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Currently `type.serialize` and `connection.{quote|type_cast}` for a time
object always does `time.getutc` call regardless of whether it is
already utc time object or not, that duplicated proccess
(`connection.type_cast(type.serialize(time))`) allocates extra/useless
time objects for each type casting.
This avoids that redundant `time.getutc` call if it is already utc time
object. In the case of a model has timestamps (`created_at` and
`updated_at`), it avoids 6,000 time objects allocation for 1,000 times
`model.save`.
```ruby
ObjectSpace::AllocationTracer.setup(%i{path line type})
pp ObjectSpace::AllocationTracer.trace {
1_000.times { User.create }
}.select { |k, _| k[0].end_with?("quoting.rb", "time_value.rb") }
```
Before (c104bfe424e6cebe9c8e85a38515327a6c88b1f8):
```
{["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
203,
:T_ARRAY]=>[1004, 0, 778, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
220,
:T_STRING]=>[2, 0, 2, 1, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
209,
:T_ARRAY]=>[8, 0, 8, 1, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
57,
:T_ARRAY]=>[4, 0, 4, 1, 1, 0],
["~/rails/activemodel/lib/active_model/type/helpers/time_value.rb",
17,
:T_DATA]=>[4000, 0, 3096, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
120,
:T_DATA]=>[2000, 0, 1548, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
126,
:T_STRING]=>[4000, 0, 3096, 0, 1, 0]}
```
After (this change):
```
{["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
203,
:T_ARRAY]=>[1004, 0, 823, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
220,
:T_STRING]=>[2, 0, 2, 1, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
209,
:T_ARRAY]=>[8, 0, 8, 1, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
57,
:T_ARRAY]=>[4, 0, 4, 1, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
126,
:T_STRING]=>[2000, 0, 1638, 0, 1, 0]}
```
|
|
|
|
|
|
|
|
|
|
|
| |
We sometimes say "✂️ newline after `private`" in a code review (e.g.
https://github.com/rails/rails/pull/18546#discussion_r23188776,
https://github.com/rails/rails/pull/34832#discussion_r244847195).
Now `Layout/EmptyLinesAroundAccessModifier` cop have new enforced style
`EnforcedStyle: only_before` (https://github.com/rubocop-hq/rubocop/pull/7059).
That cop and enforced style will reduce the our code review cost.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Currently, if `:datetime` type has a precision, type casting always does
round off subseconds regardless of whether necessary or not, it is a bit
slow.
Since #34970, `t.timestamps` with `precision: 6` by default, so
`pluck(:created_at)` in newly created app will always be affected by the
round off.
This avoids the round off if possible, it makes `pluck(:created_at)`
about 25% faster.
https://gist.github.com/kamipo/e029539f2632aee9f5e711fe66fc8842
Before (0a87d7c9ddb95cf7568baf889ff4091469ba9af4 with postgresql adapter):
```
Warming up --------------------------------------
users.pluck(:id) 344.000 i/100ms
users.pluck(:name) 336.000 i/100ms
users.pluck(:created_at) 206.000 i/100ms
Calculating -------------------------------------
users.pluck(:id) 3.620k (± 8.5%) i/s - 18.232k in 5.077316s
users.pluck(:name) 3.579k (± 9.4%) i/s - 17.808k in 5.020230s
users.pluck(:created_at) 2.069k (± 8.0%) i/s - 10.300k in 5.019284s
```
Before (0a87d7c9ddb95cf7568baf889ff4091469ba9af4 with mysql2 adapter):
```
Warming up --------------------------------------
users.pluck(:id) 245.000 i/100ms
users.pluck(:name) 240.000 i/100ms
users.pluck(:created_at) 180.000 i/100ms
Calculating -------------------------------------
users.pluck(:id) 2.548k (± 9.4%) i/s - 12.740k in 5.066574s
users.pluck(:name) 2.513k (± 8.0%) i/s - 12.480k in 5.011260s
users.pluck(:created_at) 1.771k (±11.2%) i/s - 8.820k in 5.084473s
```
After (this change with postgresql adapter):
```
Warming up --------------------------------------
users.pluck(:id) 348.000 i/100ms
users.pluck(:name) 357.000 i/100ms
users.pluck(:created_at) 254.000 i/100ms
Calculating -------------------------------------
users.pluck(:id) 3.628k (± 8.2%) i/s - 18.096k in 5.024748s
users.pluck(:name) 3.624k (±12.4%) i/s - 17.850k in 5.020959s
users.pluck(:created_at) 2.567k (± 7.0%) i/s - 12.954k in 5.081153s
```
After (this change with mysql2 adapter):
```
Warming up --------------------------------------
users.pluck(:id) 268.000 i/100ms
users.pluck(:name) 265.000 i/100ms
users.pluck(:created_at) 207.000 i/100ms
Calculating -------------------------------------
users.pluck(:id) 2.586k (±10.9%) i/s - 12.864k in 5.050546s
users.pluck(:name) 2.543k (±10.2%) i/s - 12.720k in 5.067726s
users.pluck(:created_at) 2.263k (±14.5%) i/s - 10.971k in 5.004039s
```
|
|
|
|
| |
This reverts commit 9c9c950d02af83742a5f76302d0faa99508f242c.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Before 34cc301, type casting by boolean attribute when querying is a
no-op, so finding by truthy boolean string (i.e.
`where(value: "true") # => value = 'true'`) didn't work as expected
(matches it to FALSE in MySQL #32624). By type casting is ensured, a
value on boolean attribute is always serialized to TRUE or FALSE.
In PostgreSQL, `where(value: :false) # => value = 'false'` was a valid
SQL, so 34cc301 is a regresson for PostgreSQL since all symbol values
are serialized as TRUE.
I'd say using `:false` is mostly a developer's mistake (user's input
basically comes as a string), but `:false` on boolean attribute is
serialized as TRUE is not a desirable behavior for anybody.
This allows falsy boolean symbols as false, i.e.
`klass.create(value: :false).value? # => false` and
`where(value: :false) # => value = FALSE`.
Fixes #35676.
|
|\
| |
| |
| |
| | |
kamipo/dont_allow_non_numeric_string_matches_to_zero
Don't allow `where` with non numeric string matches to 0 values
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
This is a follow-up of #35310.
Currently `Topic.find_by(id: "not-a-number")` matches to a `id = 0`
record. That is considered as silently leaking information.
If non numeric string is given to find by an integer column, it should
not be matched to any record.
Related #12793.
|
|/
|
|
|
|
|
|
|
|
|
| |
This reverts commit 52fddcc653458456f98b3683dffd781cf00b35fe.
52fddcc was to short-circuit `ensure_in_range` in `cast_value`. But that
caused a regression for empty string deserialization.
Since 7c6f393, `ensure_in_range` is moved into `serialize`. As 52fddcc
said, the absolute gain is quite small. So I've reverted that commit to
fix the regression.
|
|
|
|
|
|
|
|
|
| |
That is considered as silently leaking information.
If type casting doesn't return any actual value, it should not be
matched to any record.
Fixes #33624.
Closes #33946.
|
|\
| |
| |
| | |
Return correct date in ActiveModel for time to date conversions
|
| |
| |
| |
| |
| |
| |
| | |
time.to_date conversion happens considering leap years
so a conversion of "Day.new({'day(1i)'=>'1', 'day(2i)'=>'1', 'day(3i)'=>'1'})" results in saving the date as Mon, 03 Jan 0001
which might seem weird on the user level, hence falling back to parsing on string level resolves this data mismatch
Fixes #28521
|
| |
| |
| |
| |
| |
| |
| |
| | |
Since `serialize` is passed user input args (from `where`, schema
default, etc), a helper should provide `serialize` if the helper also
provide `cast`.
Related #32624, 34cc301, a741208.
|
| |
| |
| |
| |
| |
| |
| |
| | |
`value_from_multiparameter_assignment` defined by
`AcceptsMultiparameterTime` helper requires `default_timezone` method
which is defined at `TimeValue` helper.
Since `Date` type doesn't include `TimeValue`, I've extracted `Timezone`
helper to be shared by `Date`, `DateTime`, and `Time` types.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
When assigning a hash to a time attribute that's missing a year
component (e.g. a `time_select` with `:ignore_date` set to `true`)
then the year defaults to 1970 instead of the expected 2000. This
results in the attribute changing as a result of the save.
Before:
event = Event.new(start_time: { 4 => 20, 5 => 30 })
event.start_time # => 1970-01-01 20:30:00 UTC
event.save
event.reload
event.start_time # => 2000-01-01 20:30:00 UTC
After:
event = Event.new(start_time: { 4 => 20, 5 => 30 })
event.start_time # => 2000-01-01 20:30:00 UTC
event.save
event.reload
event.start_time # => 2000-01-01 20:30:00 UTC
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Related 34cc301f03aea2e579d6687a9ea9782afc1089a0.
`QueryAttribute#value_for_database` calls only `type.serialize`, and
`Decimal#serialize` is a no-op unlike other attribute types.
Whether or not `serialize` will invoke `cast` is undefined in our test
cases, but it actually does not work properly unless it does so for now.
|
| | |
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Since Rails 6.0 will support Ruby 2.4.1 or higher
`# frozen_string_literal: true` magic comment is enough to make string object frozen.
This magic comment is enabled by `Style/FrozenStringLiteralComment` cop.
* Exclude these files not to auto correct false positive `Regexp#freeze`
- 'actionpack/lib/action_dispatch/journey/router/utils.rb'
- 'activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb'
It has been fixed by https://github.com/rubocop-hq/rubocop/pull/6333
Once the newer version of RuboCop released and available at Code Climate these exclude entries should be removed.
* Replace `String#freeze` with `String#-@` manually if explicit frozen string objects are required
- 'actionpack/test/controller/test_case_test.rb'
- 'activemodel/test/cases/type/string_test.rb'
- 'activesupport/lib/active_support/core_ext/string/strip.rb'
- 'activesupport/test/core_ext/string_ext_test.rb'
- 'railties/test/generators/actions_test.rb'
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
For example, dirty checking was not right for the following case:
```
model.int_column = "+5"
model.float_column = "0.5E+1"
model.decimal_column = "0.5e-3"
```
It is enough to see whether leading character is a digit for avoiding
invalid numeric expression like 'wibble' to be type-casted to 0, as
this method's comment says.
Fixes #33801
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
The multiplication of the value takes a long time when we can instead mutate and use the string value directly.
The `microsec` perf increases speed by 27% in the ideal case (which is the most common).
```
original_string = ".443959"
require 'benchmark/ips'
Benchmark.ips do |x|
x.report("multiply") {
string = original_string.dup
(string.to_r * 1_000_000).to_i
}
x.report("new ") {
string = original_string.dup
if string && string.start_with?(".".freeze) && string.length == 7
string[0] = ''.freeze
string.to_i
end
}
x.compare!
end
# Warming up --------------------------------------
# multiply 125.783k i/100ms
# new 146.543k i/100ms
# Calculating -------------------------------------
# multiply 1.751M (± 3.3%) i/s - 8.805M in 5.033779s
# new 2.225M (± 2.1%) i/s - 11.137M in 5.007110s
# Comparison:
# new : 2225289.7 i/s
# multiply: 1751254.2 i/s - 1.27x slower
```
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
`QueryAttribute#value_for_database` calls only `type.serialize`, and
`Boolean#serialize` is a no-op unlike other attribute types.
It caused the issue #32624. Whether or not `serialize` will invoke
`cast` is undefined in our test cases, but it actually does not work
properly unless it does so for now.
Fixes #32624.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Since #26074, introduced force equality checking to build a predicate
consistently for both `find` and `create` (fixes #27313).
But the assumption that only array/range attribute have subtype was
wrong. We need to make force equality checking more strictly not to
allow serialized attribute.
Fixes #32761.
|
| |
| |
| |
| | |
Before it was coercing an invalid string into "2000-01-01 00:00:00".
|
| |
| |
| |
| | |
Inside user_input_in_time_zone we call in_time_zone on the value and value can be a String.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
For legacy reasons Rails stores time columns on sqlite as full
timestamp strings. However because the date component wasn't being
normalized this meant that when they were read back they were being
prefixed with 2001-01-01 by ActiveModel::Type::Time. This had a
twofold result - first it meant that the fast code path wasn't being
used because the string was invalid and second it was corrupting the
second fractional component being read by the Date._parse code path.
Fix this by a combination of normalizing the timestamps on writing
and also changing Active Model to be more lenient when detecting
whether a string starts with a date component before creating the
dummy time value for parsing.
|
| |
| |
| |
| |
| |
| |
| | |
In #20317, datetime columns had their precision applied on assignment but
that behaviour wasn't applied to time columns - this commit fixes that.
Fixes #30301.
|
| |
| |
| |
| |
| | |
https://bugs.ruby-lang.org/issues/12752
https://ruby-doc.org/core-2.4.0/String.html#method-i-unpack1
|
| |
| |
| |
| |
| |
| |
| |
| |
| | |
BC dates are supported by both date and datetime types.
https://www.postgresql.org/docs/current/static/datatype-datetime.html
Since #1097, new datetime allows year zero as 1 BC, but new date does
not. It should be allowed even in new date consistently.
|
|/ |
|
|
|
|
| |
This basically reverts ee5cfc01a5797f854c8441539b0cae326a81b963
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Between 4.2 and 5.0 the behavior of how multiparameter attributes
interact with `_before_type_cast` changed. In 4.2 it returns the
post-type-cast value. After 5.0, it returns the hash that gets sent to
the type. This behavior is correct, but will cause an issue if you then
tried to render that value in an input like `text_field` or
`hidden_field`.
In this case, we want those fields to use the post-type-cast form,
instead of the `_before_type_cast` (the main reason it uses
`_before_type_cast` at all is to avoid losing data when casting a
non-numeric string to integer).
I've opted to modify `came_from_user?` rather than introduce a new
method for this as I want to avoid complicating that contract further,
and technically the multiparameter hash didn't come from assignment, it
was constructed internally by AR.
Close #27888.
|
| |
|
|
|
|
|
| |
`ActiveModel::Type::DateTime#serialize` should return a `Time` object
so that finding by a datetime column works correctly.
|
| |
|
|
|
|
|
|
|
|
|
| |
See 34321e4a433bb7eef48fd743286601403f8f7d82 for background on
ImmutableString vs String.
Our String type cannot delegate typecasting to ImmutableString, because
the latter freezes its input: duplicating the value after that gives us
an unfrozen result, but still mutates the originally passed object.
|
|\
| |
| | |
Use `inspect` in `type_cast_for_schema` for date/time and decimal values
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Currently dumping defaults on schema is inconsistent.
Before:
```ruby
create_table "defaults", force: :cascade do |t|
t.string "string_with_default", default: "Hello!"
t.date "date_with_default", default: '2014-06-05'
t.datetime "datetime_with_default", default: '2014-06-05 07:17:04'
t.time "time_with_default", default: '2000-01-01 07:17:04'
t.decimal "decimal_with_default", default: 1234567890
end
```
After:
```ruby
create_table "defaults", force: :cascade do |t|
t.string "string_with_default", default: "Hello!"
t.date "date_with_default", default: "2014-06-05"
t.datetime "datetime_with_default", default: "2014-06-05 07:17:04"
t.time "time_with_default", default: "2000-01-01 07:17:04"
t.decimal "decimal_with_default", default: "1234567890"
end
```
|
| |
| |
| |
| | |
https://github.com/ruby/bigdecimal/pull/55
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
In Ruby 2.4, BigDecimal(), as used by the Decimal cast, was changed so
that it will raise ArgumentError when passed an invalid string, in order
to be more consistent with Integer(), Float(), etc. The other numeric
types use ex. to_i and to_f.
Unfortunately, we can't simply change BigDecimal() to to_d. String#to_d
raises errors like BigDecimal(), unlike all the other to_* methods (this
should probably be filed as a ruby bug).
Instead, this simulates the existing behaviour and the behaviour of the
other to_* methods by finding a numeric string at the start of the
passed in value, and parsing that using BigDecimal().
See also
https://bugs.ruby-lang.org/issues/10286
https://github.com/ruby/bigdecimal/commit/3081a627cebdc1fc119425c7a9f009dbb6bec8e8
|
| | |
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Right now it behaves differently on JRuby:
```
--- expected
+++ actual
@@ -1 +1 @@
-#<BigDecimal:5f3c866c,'0.333333333333333333',18(20)>
+#<BigDecimal:16e0afab,'0.3333333333333333',16(20)>
```
My initial PR (https://github.com/rails/rails/pull/27324)
offered to let the precision to be decided by the platform and
change the test expection, but other contributors suggested
that we should change the default precision in Rails
to be consistent of all platforms.
The value (18) comes from the max default precision that comes
from casting Rational(1/3) to BigDecimal.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
As pointed out by @matthewd this change makes ImmutableString aware
of MysqlString's existence whereas previously MysqlString was only
overriding public API.
cc @kamipo
This reverts commit e632c2fa4cb60072a778ce95c952a0fa95e5b074, reversing
changes made to 334a7dcf107cd3ff1697163d331d289d6d65dcd7.
|
| |
| |
| |
| |
| |
| | |
The only difference between `Type::ImmutableString` and its subclasses
is the representation of the casted booleans. Prefer extracting
`casted_true`/`casted_false` and override these by subclasses.
|
| | |
|
|/ |
|
|\
| |
| |
| | |
Moved database-specific ActiveModel types into ActiveRecord
|
| |
| |
| |
| | |
ie. DecimalWithoutScale, Text and UnsignedInteger
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Currently `apply_seconds_precision` cannnot round usec
when after `require 'mathn'`.
```
irb(main):001:0> 1234 / 1000 * 1000
=> 1000
irb(main):002:0> 1234 - 1234 % 1000
=> 1000
irb(main):003:0> require 'mathn'
=> true
irb(main):004:0> 1234 / 1000 * 1000
=> 1234
irb(main):005:0> 1234 - 1234 % 1000
=> 1000
```
|
|/ |
|
|\
| |
| | |
Print the proper ::Float::INFINITY value when used as a default value
|