Skip to content

Commit

Permalink
Dash comments should match read query regexp
Browse files Browse the repository at this point in the history
As well as allowing queries of the form

    /* some comment */
    SELECT ...

we should also allow queries of the form

    -- some comment
    SELECT ...

Additionally, I've added tests for both comment forms into the general
adapter test case, along with some additional checks to ensure that the
regexp does not return a false-positive match if the read query
signifier keyword is actually a part of the comment.

This expands on the change already implemented in
12b964c.
  • Loading branch information
lazyatom authored and eileencodes committed Oct 28, 2020
1 parent 9fb4ce4 commit d44b628
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 1 deletion.
4 changes: 4 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
* Allow double-dash comment syntax when querying read-only databases

*James Adam*

* Add `values_at` method.

Returns an array containing the values associated with the given methods.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class AbstractAdapter
include Savepoints

SIMPLE_INT = /\A\d+\z/
COMMENT_REGEX = %r{/\*(?:[^\*]|\*[^/])*\*/}m
COMMENT_REGEX = %r{(?:\-\-.*\n)*|/\*(?:[^\*]|\*[^/])*\*/}m

attr_accessor :pool
attr_reader :visitor, :owner, :logger, :lock
Expand Down
66 changes: 66 additions & 0 deletions activerecord/test/cases/adapter_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,72 @@ def test_doesnt_error_when_a_select_query_is_called_while_preventing_writes
end
end

def test_doesnt_error_when_a_select_query_starting_with_a_slash_star_comment_is_called_while_preventing_writes
@connection.insert("INSERT INTO subscribers(nick) VALUES ('138853948594')")

@connection_handler.while_preventing_writes do
result = @connection.select_all("/* some comment */ SELECT subscribers.* FROM subscribers WHERE nick = '138853948594'")
assert_equal 1, result.length
end
end

def test_errors_when_an_insert_query_prefixed_by_a_slash_star_comment_is_called_while_preventing_writes
@connection_handler.while_preventing_writes do
@connection.transaction do
assert_no_queries do
assert_raises(ActiveRecord::ReadOnlyError) do
@connection.insert("/* some comment */ INSERT INTO subscribers(nick) VALUES ('138853948594')", nil, false)
end
end
end
end
end

def test_doesnt_error_when_a_select_query_starting_with_double_dash_comments_is_called_while_preventing_writes
@connection.insert("INSERT INTO subscribers(nick) VALUES ('138853948594')")

@connection_handler.while_preventing_writes do
result = @connection.select_all("-- some comment\n-- comment about INSERT\nSELECT subscribers.* FROM subscribers WHERE nick = '138853948594'")
assert_equal 1, result.length
end
end

def test_errors_when_an_insert_query_prefixed_by_a_double_dash_comment_is_called_while_preventing_writes
@connection_handler.while_preventing_writes do
@connection.transaction do
assert_no_queries do
assert_raises(ActiveRecord::ReadOnlyError) do
@connection.insert("-- some comment\nINSERT INTO subscribers(nick) VALUES ('138853948594')", nil, false)
end
end
end
end
end

def test_errors_when_an_insert_query_prefixed_by_a_slash_star_comment_containing_read_command_is_called_while_preventing_writes
@connection_handler.while_preventing_writes do
@connection.transaction do
assert_no_queries do
assert_raises(ActiveRecord::ReadOnlyError) do
@connection.insert("/* SELECT */ INSERT INTO subscribers(nick) VALUES ('138853948594')", nil, false)
end
end
end
end
end

def test_errors_when_an_insert_query_prefixed_by_a_double_dash_comment_containing_read_command_is_called_while_preventing_writes
@connection_handler.while_preventing_writes do
@connection.transaction do
assert_no_queries do
assert_raises(ActiveRecord::ReadOnlyError) do
@connection.insert("-- SELECT\nINSERT INTO subscribers(nick) VALUES ('138853948594')", nil, false)
end
end
end
end
end

if ActiveRecord::Base.connection.supports_common_table_expressions?
def test_doesnt_error_when_a_read_query_with_a_cte_is_called_while_preventing_writes
@connection.insert("INSERT INTO subscribers(nick) VALUES ('138853948594')")
Expand Down

0 comments on commit d44b628

Please sign in to comment.