Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Support `ActiveRecord::ReadOnlyError` #1898

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
1 participant
@yahonda
Copy link
Collaborator

commented Jul 1, 2019

Refer rails/rails#34505 rails/rails#34632

$ cd activerecord
$ ARCONN=oracle bin/test test/cases/adapter_test.rb test/cases/base_test.rb -n /preventing_writes/
Using oracle
Run options: -n /preventing_writes/ --seed 62121

# Running:

.F

Failure:
ActiveRecord::AdapterTest#test_errors_when_a_delete_query_is_called_while_preventing_writes [/home/yahonda/git/rails/activerecord/test/cases/adapter_test.rb:206]:
ActiveRecord::ReadOnlyError expected but nothing was raised.


bin/test test/cases/adapter_test.rb:203

F

Failure:
ActiveRecord::AdapterTest#test_errors_when_an_update_query_is_called_while_preventing_writes [/home/yahonda/git/rails/activerecord/test/cases/adapter_test.rb:192]:
ActiveRecord::ReadOnlyError expected but nothing was raised.


bin/test test/cases/adapter_test.rb:189

F

Failure:
ActiveRecord::AdapterTest#test_errors_when_an_insert_query_is_called_while_preventing_writes [/home/yahonda/git/rails/activerecord/test/cases/adapter_test.rb:178]:
ActiveRecord::ReadOnlyError expected but nothing was raised.


bin/test test/cases/adapter_test.rb:177

.F

Failure:
BasicsTest#test_preventing_writes_with_multiple_handlers [/home/yahonda/git/rails/activerecord/test/cases/base_test.rb:1594]:
ActiveRecord::ReadOnlyError expected but nothing was raised.


bin/test test/cases/base_test.rb:1591

F

Failure:
BasicsTest#test_updating_a_record_raises_if_preventing_writes [/home/yahonda/git/rails/activerecord/test/cases/base_test.rb:1521]:
ActiveRecord::ReadOnlyError expected but nothing was raised.


bin/test test/cases/base_test.rb:1518

F

Failure:
BasicsTest#test_creating_a_record_raises_if_preventing_writes [/home/yahonda/git/rails/activerecord/test/cases/base_test.rb:1509]:
ActiveRecord::ReadOnlyError expected but nothing was raised.


bin/test test/cases/base_test.rb:1508

F

Failure:
BasicsTest#test_an_empty_transaction_does_not_raise_if_preventing_writes [/home/yahonda/git/rails/activerecord/test/cases/base_test.rb:1560]:
1 instead of 2 queries were executed.
Queries:
SAVEPOINT active_record_1.
Expected: 2
  Actual: 1


bin/test test/cases/base_test.rb:1558

F

Failure:
BasicsTest#test_preventing_writes_applies_to_all_connections_on_a_handler [/home/yahonda/git/rails/activerecord/test/cases/base_test.rb:1569]:
ActiveRecord::ReadOnlyError expected but nothing was raised.


bin/test test/cases/base_test.rb:1568

.F

Failure:
BasicsTest#test_deleting_a_record_raises_if_preventing_writes [/home/yahonda/git/rails/activerecord/test/cases/base_test.rb:1533]:
ActiveRecord::ReadOnlyError expected but nothing was raised.


bin/test test/cases/base_test.rb:1530

F

Failure:
BasicsTest#test_an_explain_query_does_not_raise_if_preventing_writes [/home/yahonda/git/rails/activerecord/test/cases/base_test.rb:1554]:
3 instead of 2 queries were executed.
Queries:
SELECT "BIRDS".* FROM "BIRDS" WHERE "BIRDS"."NAME" = :a1
EXPLAIN PLAN FOR SELECT "BIRDS".* FROM "BIRDS" WHERE "BIRDS"."NAME" = :a1
SELECT * FROM TABLE(DBMS_XPLAN.DISPLAY).
Expected: 2
  Actual: 3


bin/test test/cases/base_test.rb:1550



Finished in 41.900314s, 0.3103 runs/s, 0.4296 assertions/s.
13 runs, 18 assertions, 10 failures, 0 errors, 0 skips
$
@yahonda

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 1, 2019

One of these failures are due to SAVEPOINT statement executed, which is NOT a write query.

  • test_errors_when_a_delete_query_is_called_while_preventing_writes fails with print debug code
$ ARCONN=oracle bin/test test/cases/adapter_test.rb -n test_errors_when_a_delete_query_is_called_while_preventing_writes
Using oracle
Run options: -n test_errors_when_a_delete_query_is_called_while_preventing_writes --seed 60387

# Running:

"=========================================="
"preventing_writes?: true"
"write_query?: false"
"sql: SAVEPOINT active_record_1"
"1111111111111111111111"
F

Failure:
ActiveRecord::AdapterTest#test_errors_when_a_delete_query_is_called_while_preventing_writes [/home/yahonda/git/rails/activerecord/test/cases/adapter_test.rb:206]:
ActiveRecord::ReadOnlyError expected but nothing was raised.


bin/test test/cases/adapter_test.rb:203



Finished in 0.228083s, 4.3844 runs/s, 4.3844 assertions/s.
1 runs, 1 assertions, 1 failures, 0 errors, 0 skips
$
  • Print debug in Oracle enhanced adapter
$ git diff
diff --git a/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb b/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb
index 085b317b..09b5811b 100644
--- a/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb
+++ b/lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb
@@ -17,6 +17,10 @@ module ActiveRecord

         # Executes a SQL statement
         def execute(sql, name = nil)
+          p '=========================================='
+          p "preventing_writes?: #{preventing_writes?}"
+          p "write_query?: #{write_query?(sql)}"
+          p "sql: #{sql}"
           if preventing_writes? && write_query?(sql)
             raise ActiveRecord::ReadOnlyError, "Write query attempted while in readonly mode: #{sql}"
           end
$
  • Print debug code in Active Record
+++ b/activerecord/test/cases/adapter_test.rb
@@ -207,6 +207,7 @@ def test_errors_when_a_delete_query_is_called_while_preventing_writes
         assert_raises(ActiveRecord::ReadOnlyError) do
           @connection_handler.while_preventing_writes do
             @connection.transaction do
+              p '1111111111111111111111'
               @connection.delete("DELETE FROM subscribers WHERE nick = '138853948594'")
             end
           end
$
@yahonda

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 1, 2019

  • The same test pass with mysql2 adapter

Here the first statement is DELETE, which is an expected one.

"=========================================="
"preventing_writes?: true"
"write_query?: true"
"sql: DELETE FROM subscribers WHERE nick = '138853948594'"
"=========================================="
$ ARCONN=mysql2 bin/test test/cases/adapter_test.rb -n test_errors_when_a_delete_query_is_called_while_preventing_writes
Using mysql2
"=========================================="
"preventing_writes?: false"
"write_query?: false"
"sql: SET NAMES utf8mb4 COLLATE utf8mb4_unicode_ci,  @@SESSION.sql_mode = CONCAT(CONCAT(@@sql_mode, ',STRICT_ALL_TABLES'), ',NO_AUTO_VALUE_ON_ZERO'),  @@SESSION.sql_auto_is_null = 0, @@SESSION.wait_timeout = 2147483"
Run options: -n test_errors_when_a_delete_query_is_called_while_preventing_writes --seed 65422

# Running:

"=========================================="
"preventing_writes?: false"
"write_query?: false"
"sql: BEGIN"
"=========================================="
"preventing_writes?: false"
"write_query?: false"
"sql: BEGIN"
"=========================================="
"preventing_writes?: false"
"write_query?: true"
"sql: INSERT INTO subscribers(nick) VALUES ('138853948594')"
"1111111111111111111111"
"=========================================="
"preventing_writes?: true"
"write_query?: true"
"sql: DELETE FROM subscribers WHERE nick = '138853948594'"
"=========================================="
"preventing_writes?: false"
"write_query?: false"
"sql: ROLLBACK"
"=========================================="
"preventing_writes?: false"
"write_query?: false"
"sql: ROLLBACK"
.

Finished in 0.010384s, 96.3020 runs/s, 192.6040 assertions/s.
1 runs, 2 assertions, 0 failures, 0 errors, 0 skips
$
$ git diff
diff --git a/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb b/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb
index bbcdc96cdc..4b4a24c68a 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb
@@ -37,6 +37,10 @@ def explain(arel, binds = [])

         # Executes the SQL statement in the context of this connection.
         def execute(sql, name = nil)
+          p '=========================================='
+          p "preventing_writes?: #{preventing_writes?}"
+          p "write_query?: #{write_query?(sql)}"
+          p "sql: #{sql}"
           if preventing_writes? && write_query?(sql)
             raise ActiveRecord::ReadOnlyError, "Write query attempted while in readonly mode: #{sql}"
           end
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index 0bc617edbe..885c32be7b 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -207,6 +207,7 @@ def test_errors_when_a_delete_query_is_called_while_preventing_writes
         assert_raises(ActiveRecord::ReadOnlyError) do
           @connection_handler.while_preventing_writes do
             @connection.transaction do
+              p '1111111111111111111111'
               @connection.delete("DELETE FROM subscribers WHERE nick = '138853948594'")
             end
           end
$
@yahonda

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 1, 2019

Looks like Oracle enhanced adapter needs rails/rails#32647 support first.

Because create_savepoint method is called in the following order.

https://github.com/rails/rails/blob/60e19c6d6cb60159af4e2247f29d2cea375a21d6/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb#L234

https://github.com/rails/rails/blob/60e19c6d6cb60159af4e2247f29d2cea375a21d6/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb#L161

Oracle database itself does not need BEGIN to start a transaction, then I was aware of rails/rails#32647 but have not implemented it.

WIP
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.