Commit Graph

8757 Commits

Author SHA1 Message Date
Eugene Kenny
96a1a2a37c Use binread instead of setting file mode manually
Followup to b31022052a54ef76f347973e23519ba86ffc94ff.
2020-05-13 00:05:32 +01:00
Ryuta Kamizono
5450270782
Merge pull request #39255 from kamipo/fix_min_and_max_on_tz_aware_attributes
Fix `minimum` and `maximum` on time zone aware attributes
2020-05-13 03:51:54 +09:00
eileencodes
6833bf4d10
Remove implementation of unchecked_serialize
Since we're checking `serializable?` in the new `HomogeneousIn`
`serialize` will no longer raise an exception. We implemented
`unchecked_serialize` to avoid raising in these cases, but with some of
our refactoring we no longer need it.

I discovered this while trying to fix a query in our application that
was not properly serializing binary columns. I discovered that in at
least 2 of our active model types we were not calling the correct
serialization. Since `serialize` wasn't aliased to `unchecked_serialize`
in `ActiveModel::Type::Binary` and `ActiveModel::Type::Boolean` (I
didn't check others but pretty sure all the AM Types are broken) the SQL
was being treated as a `String` and not the correct type.

This caused Rails to incorrectly query by string values. This is
problematic for columns storing binary data like our emoji columns at
GitHub. The test added here is an example of how the Binary type was
broken previously. The SQL should be using the hex values, not the
string value of "🥦" or other emoji.

We still have the problem `unchecked_serialize` was supposed to fix -
that `serialize` shouldn't validate data, just convert it. We'll be
fixing that in a followup PR so for now we should use `serialize` so we
know all the values are going through the right serialization for their
SQL.
2020-05-12 13:37:22 -04:00
Ryuta Kamizono
11751b2ea2 Fix minimum and maximum on time zone aware attributes
This is the opposite direction of #39039.

#39111 fixes `minimum` and `maximum` on date columns with type casting
by column type on the database. But column type has no information for
time zone aware attributes, it means that attribute type should always
be precedence over column type. I've realized that fact in the related
issue report #39248.

I've reverted the expectation of #39039, to make time zone aware
attributes works.
2020-05-13 02:26:55 +09:00
Adam Lassek
3a8eee9117 Add support for PostgreSQL contains and overlaps operators
Per [this discussion][arel-discussion] on the discourse forum, this is
an addition to Arel for supporting `@>` (contains) and `&&` (overlaps)
operators in PostgreSQL. They are useful for GIN-indexed data such as a
`jsonb` or array column.

  [arel-discussion]: https://discuss.rubyonrails.org/t/what-has-happened-to-arel/74383/51
2020-05-11 22:01:37 -05:00
Yasuo Honda
950a453dc3 Address InnerJoinAssociationTest#test_eager_load_with_string_joins failure with mysql2
`ReadOnlyTest#test_field_named_field` performs implicit commit the transaction by `ReadOnlyTest#setup`
because of the MySQL database behavior.

This commit addresses the failure at https://buildkite.com/rails/rails/builds/68962#68213887-1cef-4f76-9c95-aebc8799c806
Here are minimum steps to reproduce:

```ruby
% ARCONN=mysql2 bin/test test/cases/readonly_test.rb test/cases/dirty_test.rb test/cases/associations/inner_join_association_test.rb \
-n "/^(?:ReadOnlyTest#(?:test_has_many_with_through_is_not_implicitly_marked_readonly)|DirtyTest#(?:test_field_named_field)|InnerJoinAssociationTest#(?:test_eager_load_with_string_joins))$/" --seed 50855
Using mysql2
Run options: -n "/^(?:ReadOnlyTest#(?:test_has_many_with_through_is_not_implicitly_marked_readonly)|DirtyTest#(?:test_field_named_field)|InnerJoinAssociationTest#(?:test_eager_load_with_string_joins))$/" --seed 50855

..F

Failure:
InnerJoinAssociationTest#test_eager_load_with_string_joins [/Users/yahonda/src/github.com/rails/rails/activerecord/test/cases/associations/inner_join_association_test.rb:87]:
Expected: 3
  Actual: 4

bin/test test/cases/associations/inner_join_association_test.rb:82

Finished in 0.114674s, 26.1611 runs/s, 26.1611 assertions/s.
3 runs, 3 assertions, 1 failures, 0 errors, 0 skips
```

References:
- "13.3.3 Statements That Cause an Implicit Commit"
https://dev.mysql.com/doc/refman/8.0/en/implicit-commit.html
2020-05-11 08:15:36 +09:00
akinomaeni
cdc1cb50dc Fix incorrectly successful datetime precision tests
datetime with precision was passing assert_no_microsecond_precision
unintentionally, because `/\d\z/` is match to both datetime with
precision and datetime without precision. Fixed that by changing
time and regex to make it easier to grasp with and without precision.

Fix stub_version to consider schema_cache. ref: #35795

Remove failing test for unsupported version of MariaDB. ref: fb6743a
2020-05-10 23:31:38 +09:00
Ryuta Kamizono
11f420478c Support ALGORITHM = INSTANT DDL option for index operations on MySQL
Since MySQL 8.0.12, MySQL supports `ALGORITHM=INSTANT` DDL option.

https://dev.mysql.com/doc/refman/8.0/en/innodb-online-ddl-operations.html#online-ddl-index-operations
2020-05-10 07:22:02 +09:00
Ryuta Kamizono
981299e3d2 Eager generate relation methods if a method is on Kernel
Follow up of #34122.

Relation method call is relying on method_missing, but if `Kernel` has
the same named method (e.g. `open`, etc), it will invoke Kernel's method
since method_missing is not happened.

To prevent that, eager generate relation methods if a method is the same
name on `Kernel`.

Fixes #39195.
2020-05-10 06:14:29 +09:00
Ryuta Kamizono
67b369781e Active Record uses singleton class eval in tests 2020-05-10 03:43:05 +09:00
Ryuta Kamizono
2c1b012ad7 Refactor index creation to use index definition visitor
Current SQL generation code is hard to maintenance, I've found a bug
that create index comment in bulk change table when I'm refactoring
that.
2020-05-10 02:01:15 +09:00
Ryuta Kamizono
fc7eab048b Fix index creation to preserve comment in bulk change table on MySQL
I've found the bug when I'm refactoring index creation code in #39203.
2020-05-10 02:01:15 +09:00
Ryuta Kamizono
47779e75b9 Support kwargs for named scopes
We fixed `generate_relation_method` to address kwargs warnings at
#38038, but I missed generated named scopes also need the same fix.

Test case has picked from #39196.

Co-authored-by: John Hawthorn <john@hawthorn.email>
2020-05-09 06:25:43 +09:00
Ryuta Kamizono
7ce3451927 Add test case for generate_relation_method
Positional hash argument should not be dup-ed.
2020-05-09 02:46:07 +09:00
Ryuta Kamizono
7e96bbc05b remove_foreign_key doesn't care :validate option if database has no feature
Fixes #39170.

#39170 is a regression caused by 362348b to maintain kwargs flag to
address kwargs warnings.
2020-05-08 20:55:04 +09:00
Ryuta Kamizono
82b66fe48e Fix the result of aggregations to maintain duplicated "group by" fields
Actually that result is odd and hard to predictable result to me, but we
should not change the public behavior without deprecation cycle.

I had not intended to break any apps, so I've restored the behavior.

Fixes #39171.
2020-05-07 11:53:01 +09:00
Ryuta Kamizono
b385afc5f6
Merge pull request #39172 from kamipo/allow_unscope_table_qualified_value
Allow `unscope` to be aware of table name qualified values
2020-05-07 11:28:11 +09:00
Rafael França
0f60146c8f
Merge pull request #39169 from jonathanhefner/add-date_formats-inspect
Add DATE_FORMATS[:inspect]
2020-05-06 21:40:15 -04:00
Ryuta Kamizono
5136277f44 Allow unscope to be aware of table name qualified values
It is possible to unscope only the column in the specified table.

```ruby
posts = Post.joins(:comments).group(:"posts.hidden")
posts = posts.where("posts.hidden": false, "comments.hidden": false)

posts.count
# => { false => 10 }

# unscope both hidden columns
posts.unscope(where: :hidden).count
# => { false => 11, true => 1 }

# unscope only comments.hidden column
posts.unscope(where: :"comments.hidden").count
# => { false => 11 }
```

Co-authored-by: Slava Korolev <korolvs@gmail.com>
2020-05-07 10:28:55 +09:00
Eugene Kenny
d22216f262
Merge pull request #39148 from hotatekaoru/add_change_null_for_change_table
Add `change_null` for `change_table`
2020-05-07 01:10:27 +01:00
Jonathan Hefner
2b38bf6857 Add DATE_FORMATS[:inspect]
Follow-up to #39147 and #39168.

By adding a new purpose-specific format, we avoid potential pitfalls
from concatenating format strings.  We also save a String allocation per
Time attribute per inspect.

The new format also includes a time zone offset for more introspective
inspection.
2020-05-06 15:05:02 -05:00
Adam Hess
3812134049 Don't attempt to add a string to a lambda
DATE_FORMATS can be either a format string or a lambda we don't want to attempt to concat them with a string if we are in the lambda case
2020-05-06 11:43:24 -07:00
Ryuta Kamizono
46a22ceaff
Merge pull request #39162 from kamipo/dogfooding_symbol_starts_ends_with
Dogfooding "active_support/core_ext/symbol/starts_ends_with"
2020-05-06 17:19:19 +09:00
Xavier Noria
afabe994e0 Removes require_dependency from the AR test suite
In the AR test suite require_dependency does not make much sense. Just
call vanilla require/load.

Note that in the test that made explicit use of it, there are no
autoload paths, and no constants have been autoloaded. In reality, the
code ended up calling Kernel#load.
2020-05-06 09:01:38 +02:00
Ryuta Kamizono
98a1405f07 Dogfooding "active_support/core_ext/symbol/starts_ends_with"
Any missing thing would be found such like #39159.
2020-05-06 14:19:25 +09:00
Ryuta Kamizono
5cb261af8e Ensure type cast is not evaluated at relation build time
Some apps would expect that type cast is not evaluated at relation build
time consistently.

Context:

b571c4f3f2 (r31015008)
https://github.com/rails/rails/pull/34303

And also this removes extra grouping on IN clause behaved as before.
2020-05-06 12:50:24 +09:00
Jonathan Hefner
c806be33f7 Ensure sqlite3_mem transaction tests run in memory
SQLite3 does not recognize paths as file URIs unless the
`SQLite3::Constants::Open::URI` flag is set.  Therefore, without this
flag, a path like "file::memory:" is interpreted as a filename, causing
a "file::memory:" file to be created and used as the database.  Most
tests in `SQLite3TransactionTest` picked up this flag from
`shared_cache_flags`, but a few did not.  Because those tests were
creating a file, the path was changed in #38620 such that it no longer
pointed to an in-memory database.

This commit restores the database path as "file::memory:" and ensures
the URI flag is set whenever `in_memory_db?` is true.
2020-05-05 19:59:35 -05:00
Ryuta Kamizono
42daf01958
Merge pull request #39156 from bogdan/preloader-duplicate-object-ids
Ensure array passed to preloader has no duplicate records by object_id
2020-05-06 02:48:48 +09:00
Bogdan Gusiev
3d76110f63 Ensure array passed to preloader has no duplicate records by object_id
Fixes #39073
2020-05-05 20:30:13 +03:00
eileencodes
200058b011
Revert "Merge pull request #39022 from kamipo/perf_where_in"
This reverts commit 9817d74f3be72d8e685301bfd0acb6a12b9cdda9, reversing
changes made to d326b029e0d3cd649d80a484ceb5138475d3601d.

Just making this easier to merge our PR in. Otherwise there's tons of
conflicts and our PR is faster.
2020-05-05 12:45:10 -04:00
Ryuta Kamizono
57b4668c11 save and save! doesn't take positional arguments
Some commits adds `**` to address kwargs warnings in Ruby 2.7, but
`save` and `save!` are originally doesn't take positional arguments, so
maintain both `*` and `**` is redundant.

6d68bb5f69
09d7ce7975
51a7422c9f
2020-05-05 16:51:21 +09:00
Ryuta Kamizono
9817d74f3b
Merge pull request #39022 from kamipo/perf_where_in
PERF: Improve performance of where when using an array of values
2020-05-05 12:06:35 +09:00
Ryuta Kamizono
d326b029e0
Merge pull request #39051 from kamipo/more_concise_or_ast
More concise Arel `Or` ast and make `Or` visitor non recursive
2020-05-05 12:05:21 +09:00
Ryuta Kamizono
6187b7138c Fix rewhere to truly overwrite collided where clause by new where clause
```ruby
steve = Person.find_by(name: "Steve")
david = Author.find_by(name: "David")

relation = Essay.where(writer: steve)

# Before
relation.rewhere(writer: david).to_a # => []

# After
relation.rewhere(writer: david).to_a # => [david]
```

For now `rewhere` only works for truly column names, doesn't work for
alias attributes, nested conditions, associations.

To fix that, need to build new where clause first, and then get
attribute names from new where clause.
2020-05-05 11:45:29 +09:00
hotatekaoru
972d18eab5 Add change_null for change_table
To change a NOT NULL constraint `reversible`.

When changing a NOT NULL constraint, we use `ActiveRecord::ConnectionAdapters::SchemaStatements#change` method that is not reversible, so `up` and `down` methods were required. Actually, we can use `change_column_null` method if only one constraint changed, but if we want to change multiple constarints with ALTER QUERY, `up` and `down` methods were required.
2020-05-05 11:32:38 +09:00
Eugene Kenny
3d426492d4 Remove obsolete explain logging test
This is a remnant of the auto-explain feature, which was removed in
d3688e02ca52c0b72d3092e8498da51e06b7fc58.
2020-05-05 02:24:53 +01:00
Eugene Kenny
072a962fa2
Merge pull request #39138 from jonathanhefner/flakey-destroyed_by_association-tests
Fix flakey destroyed_by_association tests
2020-05-05 01:51:09 +01:00
Jonathan Hefner
7f5deeeee6 Fix flakey destroyed_by_association tests
Example failure: https://buildkite.com/rails/rails/builds/68661#84f8790a-fc9e-42ef-a7fb-5bd15a489de8/1002-1012

The failing `destroyed_by_association` tests create an author (a
DestroyByParentAuthor) and a book (a DestroyByParentBook) that belongs
to that author.  If the database already contains books that refer to
that author's ID from previous tests (i.e. tests that disabled
`use_transactional_tests`), then one of those books will be loaded and
destroyed instead of the intended DestroyByParentBook book.

By loading the `:books` fixtures, we ensure the database does not
contain such unexpected books.

Co-authored-by: Eugene Kenny <elkenny@gmail.com>
Co-authored-by: Ryuta Kamizono <kamipo@gmail.com>
2020-05-04 19:26:54 -05:00
akinomaeni
a3f4202cd8 Inspect time attributes with subsec
before
```
p Knot.create
=> #<Knot id: 1, created_at: "2016-05-05 01:29:47">
```

after
```
p Knot.create
=> #<Knot id: 1, created_at: "2016-05-05 01:29:47.116928000">
```
2020-05-05 08:32:20 +09:00
Ryuta Kamizono
ed6da7038a Test actual query and result 2020-05-04 21:55:27 +09:00
Ryuta Kamizono
7d4cc56ef8 Fix rewhere to allow overwriting association queries 2020-05-04 20:05:41 +09:00
Eugene Kenny
8fd418955e Allow associations to be autosaved multiple times
Since 901d62c586c20ab38b0f18f4bd9a4419902768c4, associations can only be
autosaved once: after a record has been saved, `@new_record_before_save`
will always be false. This assumes that records only transition to being
persisted once, but there are two cases where it happens multiple times:
when the transaction that saved the record is rolled back, and when the
persisted record is later duplicated.
2020-05-03 08:55:30 +01:00
Ryuta Kamizono
483330eeea
Merge pull request #39111 from kamipo/fix_min_and_max_on_non_numeric_column
Fix `minimum` and `maximum` on non numeric column
2020-05-02 15:11:34 +09:00
Ryuta Kamizono
767edafc69 Fix minimum and maximum on non numeric column
I supposed all aggregation functions will return numeric result in
#39039, but that assumption was incorrect for `minimum` and `maximum`,
if an aggregated column is non numeric type.

I've restored type casting aggregated result for `minimum` and `maximum`.

Fixes #39110.
2020-05-02 14:39:39 +09:00
Ryuta Kamizono
92360e9ea3 Deprecate passing a column to type_cast
The type information for type casting is entirely separated to type
object, so if anyone does passing a column to `type_cast` in Rails 6,
they are likely doing something wrong. See the comment for more details:

28d815b894/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb (L33-L42)

This also deprecates passing legacy binds (an array of `[column, value]`
which is 4.2 style format) to query methods on connection. That legacy
format was kept for backward compatibility, instead of that, I've
supported casted binds format (an array of casted values), it is easier
to construct binds than existing two binds format.
2020-05-02 07:18:30 +09:00
Ryuta Kamizono
7669dc2196 Support limit(n) and offset(n) nodes in dot output 2020-05-02 07:14:17 +09:00
Ryuta Kamizono
3754088685
Merge pull request #39101 from kamipo/remove_without_transaction_enrollment_callbacks
Remove internal `without_transaction_enrollment` callbacks
2020-05-02 05:26:43 +09:00
Ryuta Kamizono
e54fd4e052
Merge pull request #39097 from kamipo/improve_find_by_sql
Avoid quite useless loop in `find_by_sql`
2020-05-02 05:25:19 +09:00
eileencodes
b74256be5b
Add test for slug to ID with ID is out of range
We were testing the behavior of out of range ID's on where queries but
not when out of range slugs are converted to ID's.
2020-05-01 12:45:29 -04:00
Ryuta Kamizono
75873d8f5f Remove internal without_transaction_enrollment callbacks
I've found the internal `without_transaction_enrollment` callbacks which
have not been newly used over five years, when I tried to work reverting
#9068 (https://github.com/rails/rails/pull/36049#issuecomment-487318060).

I think that we will never make that callbacks public, since the
mechanism of `without_transaction_enrollment` is too implementation
specific, at least before #9068, records in a transaction had enrolled
all into the transaction.

That callbacks was introduced at #18936 to make `touch_later` #19324,
but I think that the internal callbacks is overkill to just make the
`touch_later` only, and invoking the extra callbacks also have a little
overhead even if we haven't used that.

So I think we can remove the internal callbacks for now, until we will
come up with a good use for that callbacks.
2020-05-01 23:32:26 +09:00