Skip to content

Commit

Permalink
fix: SpannerAdapter requires prepared statements to be enabled (#323)
Browse files Browse the repository at this point in the history
* fix: `SpannerAdapter` requires prepared statements to be enabled

The adapter relies on prepared statements to extract Spanner native
request options from the binds in the query engine call stack.

This is at odds with the query tags feature in Rails 7.1, as it disables
prepared statements for all adapters when it is enabled. This in turn

The spanner AR adapter relies on prepared statements to extract native
database functionality on the binds. The query tags feature, [when enabled](https://github.com/rails/rails/blob/7-1-stable/activerecord/lib/active_record/railtie.rb#L419),
[disables prepared statements](https://github.com/rails/rails/blob/main/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb#L160-L162),
breaking the Spanner adapters queries, like inserts and updates.

* test: verify that prepared statement is used

* fix: only disable prepared statements if supported

---------

Co-authored-by: Knut Olav Løite <[email protected]>
  • Loading branch information
cbarton and olavloite authored Dec 10, 2024
1 parent e3c77c6 commit 4896440
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 0 deletions.
3 changes: 3 additions & 0 deletions lib/active_record/connection_adapters/spanner_adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ def initialize connection, logger, connection_options, config
@connection_options = connection_options
super connection, logger, config
@raw_connection ||= connection

# Spanner does not support unprepared statements
@prepared_statements = true
end

def max_identifier_length
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,39 @@ def test_insert!
assert_equal "id", mutation.insert.columns[2]
end

def test_insert_with_disabled_prepared_statements
if ActiveRecord.respond_to?(:disable_prepared_statements)
ActiveRecord.disable_prepared_statements = true
ActiveRecord::Base.establish_connection(
adapter: "spanner",
emulator_host: "localhost:#{@port}",
project: "test-project",
instance: "test-instance",
database: "testdb",
)
assert ActiveRecord::Base.connection.prepared_statements?
end

insert_sql = "INSERT INTO `singers` (`first_name`, `last_name`, `id`) VALUES (@p1, @p2, @p3)"
@mock.put_statement_result insert_sql, StatementResult.new(1)
ActiveRecord::Base.transaction do
singer = { first_name: "Alice", last_name: "Ecila" }
Singer.create singer
end

requests = @mock.requests
request = requests.select { |req| req.is_a?(ExecuteSqlRequest) && req.sql == insert_sql }.first
assert_equal "Alice", request.params["p1"]
assert_equal "Ecila", request.params["p2"]
assert_equal :STRING, request.param_types["p1"].code
assert_equal :STRING, request.param_types["p2"].code
assert_equal :INT64, request.param_types["p3"].code
ensure
if ActiveRecord.respond_to?(:disable_prepared_statements)
ActiveRecord.disable_prepared_statements = false
end
end

def test_upsert
singer = { first_name: "Alice", last_name: "Ecila" }
singer = Singer.upsert singer
Expand Down

0 comments on commit 4896440

Please sign in to comment.