Commit Graph

18650 Commits

Author SHA1 Message Date
Ryuta Kamizono 7be2a37bc9 Remove useless `update_attributes_from_transaction_state` and `set_transaction_state` 2019-04-14 21:58:52 +09:00
Ryuta Kamizono 533dd8a681 Don't expose `add_to_transaction`
`add_to_transaction` was added at da840d1, but it should not be called
by except internal, since `remember_transaction_record_state` should be
called only once before saving.

And also, currently `add_to_transaction` doesn't always add the record
to transaction since da8de91, that is the reason hard to use that even
in internal.

Even if `add_to_transaction` ensure to add the record to transaction,
that is an internal concern, people don't need to explicitly call
`add_to_transaction`.
2019-04-14 20:55:46 +09:00
Ryuta Kamizono 98d4a68a80
Merge pull request #35966 from shioyama/define_validator_on_topic_subclass
Add validation to subclass in tests to avoid polluting parent class
2019-04-14 01:40:42 +09:00
Ryuta Kamizono 877376eba1
Merge pull request #35958 from yskkin/bulk_change_table
use PostgreSQL's bulk_alter_table implementation
2019-04-14 01:23:39 +09:00
Chris Salzberg 42a8340aa8
Add validation to subclass in tests to avoid polluting parent class
These two tests currently both define acceptance validators on the same
class, Topic. This means that in either one test or the other, there are
not one but *two* instances of the LazilyDefineAttributes module
builder in the class' ancestors, which can result in unpredictable
results.

Subclassing Topic in each test avoids conflicts.
2019-04-13 22:20:56 +09:00
Yoshiyuki Kinjo 53f1b3e579 use PostgreSQL's bulk_alter_table implementation
Running this migration on mysql at current master fails
because `add_references_for_alter` is missing.

```
change_table :users, bulk: true do |t|
  t.references :article
end
```

This is also true for postgresql adapter,
but its `bulk_alter_table` implementation can fallback in such case.
postgresql's implementation is desirable to prevent unknown failure like this.
2019-04-13 15:39:19 +09:00
Ryuta Kamizono cc6bff3daa Auto-correct `Style/StringLiterals` cop offences
Somehow Code Climate is not working as expected for now?
2019-04-13 11:43:47 +09:00
Chris Salzberg c9d75177fe
Improve wording of comments
Most of the time, these methods are called from actual methods defined
from columns in the schema, not from method_missing, so the current
wording is misleading.
2019-04-13 11:12:39 +09:00
yuuji.yaginuma f95c132ef7 Fix `presicion` -> `precision`
This fix is necessary to test precision's default correctly.
2019-04-13 07:59:41 +09:00
Guilherme Mansur cd50e952e2 Fix test flakyness due to `test_truncate_tables`
`Truncate Tables posts` will also reset the `AUTOINCREMENT` this causes
a situation where if a test suite that uses the `taggings` fixtures runs
and subsequently the `test_truncate_tables` run, newly created posts
would reference the `tagging` in the database. This commit resest the
state of the posts table after the `connection.truncate` call in the
`test_truncate_tables`, as well as all other tests that call `trucate`
This ensures the associations and db state remain consistent after the
tests.

Fixes: https://github.com/rails/rails/issues/35941
2019-04-12 15:47:54 -04:00
Ryuta Kamizono ef0c5fe0d9
Merge pull request #35918 from kamipo/lazy_sync_with_transaction_state_on_destroy
Lazy sync with transaction state on destroy
2019-04-12 22:11:38 +09:00
Ryuta Kamizono 186566c58a
Merge pull request #35920 from kamipo/dont_call_commit_callbacks_for_invalid_record
Don't call after_commit callbacks despite a record isn't saved
2019-04-12 22:10:04 +09:00
Ryuta Kamizono 8c24908682
Merge pull request #28830 from kamipo/dont_regard_extension_block_as_scope
Fix `automatic_inverse_of` not to be disabled if extension block is given
2019-04-12 22:09:12 +09:00
Ryuta Kamizono 59d32a6eab Refactor around sql_type metadata and column
* remove useless `@type_metadata` and `@array`
* move the compatibility code (for array) into column
* etc.
2019-04-12 22:03:23 +09:00
Ryuta Kamizono fa8c525d20 Fix `automatic_inverse_of` not to be disabled if extension block is given
If an association has a scope, `automatic_inverse_of` is to be disabled.
But extension block is obviously not a scope. It should not be regarded
as a scope.

Fixes #28806.
2019-04-12 18:01:57 +09:00
Ryuta Kamizono 9252da9659 Don't call after_commit callbacks despite a record isn't saved
Regardless of a record isn't saved (e.g. validation is failed),
`after_commit` / `after_rollback` callbacks are invoked for now.

To fix the issue, this adds a record to the current transaction only
when a record is actually saved.

Fixes #29747.
Closes #29833.
2019-04-12 09:19:03 +09:00
Rafael França 032d4ad42c
Merge pull request #35838 from yahonda/more_than_1000_inlist
Address `ORA-01795: maximum number of expressions in a list is 1000`
2019-04-11 18:42:48 -04:00
Rafael França 644cd445c5
Merge pull request #35921 from Shopify/deduplicate-activerecord-strings
Deduplicate Active Record reflection names
2019-04-11 17:51:11 -04:00
Rafael França d8ab7ef179
Merge pull request #35922 from michaelglass/move-sqlite-3-database-statements-into-database-statements
make SQLite3 `last_inserted_id` private and organize DatabaseStatement methods
2019-04-11 17:50:29 -04:00
Ryuta Kamizono c820d8d7da
Merge pull request #35933 from kamipo/refactor_dirty_tracking
PERF: 2x ~ 30x faster dirty tracking
2019-04-12 04:39:54 +09:00
Yasuo Honda 029d05fb43 Address `ORA-01795: maximum number of expressions in a list is 1000`
* To address this error, this commit splits expressions by slices of 1000 elements.

* "Oracle Database Error Messages 18c"
https://docs.oracle.com/en/database/oracle/oracle-database/18/errmg/

```
ORA-01795: maximum number of expressions in a list is 1000
Cause: Number of expressions in the query exceeded than 1000. Note that unused column/expressions are also counted Maximum number of expressions that are allowed are 1000.
```

* This commit addresses this ORA-01795 error

Note: Actually addressing this error raises another "ORA-00913: too many values"
Number of values Oracle database allows is 65535 regardless bind values or literal values.

```ruby
$ ARCONN=oracle bin/test test/cases/bind_parameter_test.rb -n test_too_many_binds

... snip ...

Error:
ActiveRecord::BindParameterTest#test_too_many_binds:
ActiveRecord::StatementInvalid: OCIError: ORA-01795: maximum number of expressions in a list is 1000
    stmt.c:267:in oci8lib_260.so
    /home/yahonda/.rbenv/versions/2.6.2/lib/ruby/gems/2.6.0/gems/ruby-oci8-2.2.7/lib/oci8/cursor.rb:131:in `exec'
    /home/yahonda/git/oracle-enhanced/lib/active_record/connection_adapters/oracle_enhanced/oci_connection.rb:142:in `exec'
    /home/yahonda/git/oracle-enhanced/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb:41:in `block in exec_query'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:676:in `block (2 levels) in log'
    /home/yahonda/.rbenv/versions/2.6.2/lib/ruby/2.6.0/monitor.rb:230:in `mon_synchronize'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:675:in `block in log'
    /home/yahonda/git/rails/activesupport/lib/active_support/notifications/instrumenter.rb:24:in `instrument'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:666:in `log'
    /home/yahonda/git/oracle-enhanced/lib/active_record/connection_adapters/oracle_enhanced/dbms_output.rb:36:in `log'
    /home/yahonda/git/oracle-enhanced/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb:24:in `exec_query'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:484:in `select'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:70:in `select_all'
    /home/yahonda/git/rails/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb:106:in `select_all'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation/calculations.rb:299:in `block in execute_simple_calculation'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation.rb:755:in `skip_query_cache_if_necessary'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation/calculations.rb:299:in `execute_simple_calculation'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation/calculations.rb:251:in `perform_calculation'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation/calculations.rb:141:in `calculate'
    /home/yahonda/git/rails/activerecord/lib/active_record/relation/calculations.rb:49:in `count'
    /home/yahonda/git/rails/activerecord/test/cases/bind_parameter_test.rb:113:in `test_too_many_binds'

bin/test test/cases/bind_parameter_test.rb:109

.............................F

```
2019-04-11 12:54:53 +00:00
Ryuta Kamizono 6b0a9de906 PERF: 2x ~ 30x faster dirty tracking
Currently, although using both dirty tracking (ivar backed and
attributes backed) on one model is not supported (doesn't fully work at
least), both dirty tracking are being performed, that is very slow.

As long as attributes backed dirty tracking is used, ivar backed dirty
tracking should not need to be performed.

I've refactored to extract new `ForcedMutationTracker` which only tracks
`force_change` to be performed for ivar backed dirty tracking, that
makes dirty tracking on Active Record 2x ~ 30x faster.

https://gist.github.com/kamipo/971dfe0891f0fe1ec7db8ab31f016435

Before:

```
Warming up --------------------------------------
            changed?     4.467k i/100ms
             changed     5.134k i/100ms
             changes     3.023k i/100ms
  changed_attributes     4.358k i/100ms
        title_change     3.185k i/100ms
           title_was     3.381k i/100ms
Calculating -------------------------------------
            changed?     42.197k (±28.5%) i/s -    187.614k in   5.050446s
             changed     50.481k (±16.0%) i/s -    246.432k in   5.045759s
             changes     30.799k (± 7.2%) i/s -    154.173k in   5.030765s
  changed_attributes     51.530k (±14.2%) i/s -    252.764k in   5.041106s
        title_change     44.667k (± 9.0%) i/s -    222.950k in   5.040646s
           title_was     44.635k (±16.6%) i/s -    216.384k in   5.051098s
```

After:

```
Warming up --------------------------------------
            changed?    24.130k i/100ms
             changed    13.503k i/100ms
             changes     6.511k i/100ms
  changed_attributes     9.226k i/100ms
        title_change    48.221k i/100ms
           title_was    96.060k i/100ms
Calculating -------------------------------------
            changed?    245.478k (±16.1%) i/s -      1.182M in   5.015837s
             changed    157.641k (± 4.9%) i/s -    796.677k in   5.066734s
             changes     70.633k (± 5.7%) i/s -    358.105k in   5.086553s
  changed_attributes     95.155k (±13.6%) i/s -    470.526k in   5.082841s
        title_change    566.481k (± 3.5%) i/s -      2.845M in   5.028852s
           title_was      1.487M (± 3.9%) i/s -      7.493M in   5.046774s
```
2019-04-11 16:30:40 +09:00
takakuda 91a2e4f9e3 adjust style 2019-04-11 13:27:21 +09:00
Michael Glass 6ba31a7456
moves sqlite3 methods that mirror Abstract::DatabaseStatements into Sqlite3::DatabaseStatements and make those that are private in Abstract::DatabaseStatements private for sqlite3 2019-04-10 16:06:15 +02:00
Jean Boussier 314d66b5a1 Deduplicate Active Record reflection names 2019-04-10 16:03:14 +02:00
Roberto Miranda 63ca58ae7b Adding type option example to the documentation [ci skip] (#35917)
* Adding type option example to the documentation [ci skip]

It was hard for me looking https://api.rubyonrails.org/ to find that there was a type option. 

Adding this to the doc would be helpful especially for application with old tables where the references are still an integer not bigint

* Update activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb

Co-Authored-By: robertomiranda <rjmaltamar@gmail.com>
2019-04-10 21:44:05 +09:00
Ryuta Kamizono bcb0fb7927 Lazy sync with transaction state on destroy
This reverts commit 58410b3d56.

If we have any implicit commit/rollback callbacks, it is necessary to
add record to transaction explicitly like `:touch_deferred_attributes`.

5f261d04d6/activerecord/lib/active_record/touch_later.rb (L9)
5f261d04d6/activerecord/lib/active_record/touch_later.rb (L25)

But I can't find any other implicit commit/rollback callbacks in our
code base at least now.

I think the `self.class.connection.add_transaction_record(self)` line
doesn't cover any behavior.
2019-04-10 19:26:15 +09:00
Ryuta Kamizono 5f261d04d6 Revert "Remove unused callbacks in the `Topic` model"
This reverts commit b33ccaa6c3.

That isn't hit by `git grep`, but actually used in meta-programming...

b33ccaa6c3/activerecord/test/cases/transactions_test.rb (L1020-L1028)
2019-04-10 17:59:29 +09:00
Ryuta Kamizono b33ccaa6c3 Remove unused callbacks in the `Topic` model 2019-04-10 17:53:12 +09:00
Ryuta Kamizono faf07d1468 Merge pull request #28155 from lcreid/belongs_to
Fix "autosave: true" on belongs_to of join model causes invalid records to be saved
2019-04-10 16:42:16 +09:00
Ryuta Kamizono 7cb3e8b8ef Accidentally lost `comment` in `Column#==` and `Column#hash`
Refer #35875.
2019-04-10 12:24:43 +09:00
Ryuta Kamizono 8188f10124 Add assertions for lazy sync transaction state 2019-04-10 11:42:36 +09:00
Ryuta Kamizono f21bc468c3 Remove unused `sequence_name` in `sql_for_insert`
All adapters (sqlite3, mysql2, postgresql, oracle-enhanced, sqlserver)
doesn't use `sequence_name` in `sql_for_insert`.

4e0db270a9/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb (L79-L85)
959fe8f497/lib/active_record/connection_adapters/sqlserver/database_statements.rb (L226-L249)

It can be handled in `exec_insert` like postgresql adapter if we want.
2019-04-10 10:16:36 +09:00
Ryuta Kamizono 04d30dae61 There is no need to create `QueryAttribute` to just type cast a value 2019-04-10 09:56:29 +09:00
Rafael França 8976507439
Merge pull request #35875 from Shopify/alloc-free-comparisons
Improve == and hash methods on various schema cache structs to be allocation free.
2019-04-09 16:31:31 -04:00
Dan Fitch 2e117c8a0b Clarify exists check in logs
The default log messages for Model.exists?, when called from .save
on an object which uses scoped uniqueness validation like:

    class Example < ApplicationRecord
      validates :field, uniqueness: {scope: parent_id}
    end

can result in slightly misleading logs.

An example case:

    ↳ app/controllers/example_controller.rb:23
    (0.2ms)  begin transaction
    ↳ app/controllers/example_controller.rb:39
    Example Exists (0.2ms)  SELECT  1 AS one FROM "examples" WHERE "examples"."field" IS NULL AND "examples"."parent_id" = ? LIMIT ?  [["parent_id", 123], ["LIMIT", 1]]
    ↳ app/controllers/example_controller.rb:39
    (0.1ms)  rollback transaction

To me, a Rails newbie, this parsed as the following:

- started the transaction to create a thing
- found that your object exists already!
- so we rolled back the transaction

(even though the actual cause of the transaction is something that happens
after the Exists check.)

All this does is add a question mark to the message, to make it clear in the
log that this is a check, not a confirmation.

This may be kind of silly, but it may save some future goofs by newbs like me.
2019-04-09 12:22:05 -05:00
Ryuta Kamizono dd58d040b2
Merge pull request #35909 from simi/alias-postgresql-adapter
Bring back postgresql_version as an alias.
2019-04-10 01:24:54 +09:00
Ryuta Kamizono 2ab3751781 Remove duplicated attribute alias resolution in `_select!`
This is also resolved in `arel_column`.
2019-04-09 23:11:11 +09:00
Josef Šimánek 7618d08b26
Bring back postgresql_version as an alias. 2019-04-09 15:37:00 +02:00
Ryuta Kamizono 249f2f3377 `get_database_version` is not public API [ci skip] 2019-04-09 21:39:43 +09:00
Jean Boussier 6f26e99cef Improve == and hash methods on various schema cache structs to be allocation free.
The previous implementation would allocate 2 arrays per comparisons.

I tried relying on Struct, but they do allocate one Hash inside `Struct#hash`.
2019-04-09 13:54:49 +02:00
Ryo Hashimoto ab6da0c4a8 Fix upsert method comment
Because this method only updates or inserts a single record
like `insert` method.
2019-04-09 12:06:25 +09:00
Matthew Draper 95fee9438b
Merge pull request #34800 from mqchau/mysqlCountDeleteRowInLock
Wrap Mysql count of deleted rows in lock block to avoid conflict in test
2019-04-09 09:44:04 +09:30
Ryuta Kamizono a497ece3f7
Merge pull request #35887 from kamipo/argument_error
Raise `ArgumentError` for invalid `:limit` and `:precision` like as other options
2019-04-09 04:41:28 +09:00
Ryuta Kamizono 35d388b2e9
Merge pull request #35890 from kamipo/except_table_name_from_column
Except `table_name` from column objects
2019-04-09 04:39:15 +09:00
Quan Chau 66762b81f4 Wrap Mysql count of deleted rows in lock block to avoid conflict in test 2019-04-08 12:02:41 -07:00
Eileen M. Uchitelle 27c3ad0ac0
Merge pull request #35892 from ryohashimoto/bulk_insert_logs
Improve log messages for #insert_all` / `#upsert_all` etc. methods
2019-04-08 11:26:31 -04:00
Ryo Hashimoto 54dce6870d Improve log messages for #insert_all` / `#upsert_all` / `#insert` / `#upsert etc. methods
In #35077, `#insert_all` / `#upsert_all` / `#insert` / `#upsert` etc. methods are added. But Active Record logs only “Bulk Insert” log messages when they are invoked.

This commit improves the log messages to use collect words for how invoked them.
2019-04-08 23:20:14 +09:00
Bob Lail c24efdbd27 When skipping duplicates in bulk insert on MySQL, avoid assigning id when not specified
If `id` is an `AUTONUMBER` column, then my former strategy here of assigning `no_op_column` to an arbitrary column would fail in this specific scenario:

1. `model.columns.first` is an AUTONUMBER column
2. `model.columns.first` is not assigned in the insert attributes

I added three tests: the first test covers the actual error; the second test documents that this _isn't_ a problem when a value is given for the AUTONUMBER column and the third test ensures that this no-op strategy isn't secretly doing an UPSERT.
2019-04-08 08:01:40 -05:00
Ryuta Kamizono f185e0ebc9 Except `table_name` from column objects
The `table_name` was added at #23677 to detect whether serial column or
not correctly.

We can do that detection before initialize column object, it makes
column object size smaller, and it probably helps column object
de-duplication.
2019-04-08 19:23:26 +09:00