AR does not send a semicolon after a SQL statement

Shopify · Dec 24, 2023 · 8e73b3d · 8e73b3d
1 parent 6402f77
commit 8e73b3d
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 8 deletions.
diff --git a/lib/semian/activerecord_trilogy_adapter.rb b/lib/semian/activerecord_trilogy_adapter.rb
@@ -45,7 +45,14 @@ module ActiveRecordTrilogyAdapter
     # this method to be ~20x faster in the common case vs matching the full regular expression
     class << self
       def query_allowlisted?(sql, *)
-        if !sql.end_with?("T") && !sql.end_with?("K") && !sql.end_with?("_1") && !sql.end_with?("_2")
+        # Any nesting pass _3 levels is won't get bypassed. I think that is fine once
+        # you are 3 level deep in nested transactions you have bigger problems.
+        unlikely_to_be_tx_control_statement = !sql.end_with?("T") && !sql.end_with?("K") && !sql.end_with?("_1")\
+        && !sql.end_with?("_2")
+        # ActiveRecord does not send trailing spaces of ; we are in the realm of hand crafted queries here
+        unclear = sql.end_with?(" ") || sql.end_with?(";")
+
+        if unlikely_to_be_tx_control_statement && !unclear
           false
         else
           QUERY_ALLOWLIST.match?(sql)

diff --git a/test/adapters/activerecord_trilogy_adapter_test.rb b/test/adapters/activerecord_trilogy_adapter_test.rb
@@ -306,23 +306,23 @@ def test_semian_allows_rollback
         @adapter.execute("START TRANSACTION;")
 
         Semian[:mysql_testing].acquire do
-          @adapter.execute("ROLLBACK;")
+          @adapter.execute("ROLLBACK")
         end
       end
 
       def test_semian_allows_rollback_with_marginalia
         @adapter.execute("START TRANSACTION;")
 
         Semian[:mysql_testing].acquire do
-          @adapter.execute("/*foo:bar*/ ROLLBACK;")
+          @adapter.execute("/*foo:bar*/ ROLLBACK")
         end
       end
 
       def test_semian_allows_commit
         @adapter.execute("START TRANSACTION;")
 
         Semian[:mysql_testing].acquire do
-          @adapter.execute("COMMIT;")
+          @adapter.execute("COMMIT")
         end
       end
 
@@ -334,23 +334,24 @@ def test_query_allowlisted_returns_false_for_binary_sql
 
       def test_semian_allows_release_savepoint
         @adapter.execute("START TRANSACTION;")
+        @adapter.execute("SAVEPOINT active_record_2;")
 
         Semian[:mysql_testing].acquire do
-          @adapter.execute("RELEASE SAVEPOINT active_record_99;")
+          @adapter.execute("RELEASE SAVEPOINT active_record_2")
         end
 
         @adapter.execute("ROLLBACK;")
       end
 
       def test_semian_allows_rollback_to_savepoint
         @adapter.execute("START TRANSACTION;")
-        @adapter.execute("SAVEPOINT active_record_99;")
+        @adapter.execute("SAVEPOINT active_record_1;")
 
         Semian[:mysql_testing].acquire do
-          @adapter.execute("ROLLBACK TO SAVEPOINT active_record_99;")
+          @adapter.execute("ROLLBACK TO SAVEPOINT active_record_1")
         end
 
-        @adapter.execute("ROLLBACK;")
+        @adapter.execute("ROLLBACK")
       end
 
       def test_changes_timeout_when_half_open_and_configured