Skip to content

Commit

Permalink
Fix inserts on MySQL with no RETURNING support for a table with mul…
Browse files Browse the repository at this point in the history
…tiple auto populated columns

c2c861f wasn't a sufficient fix for tables that
have more than just one auto populated column but the database doesn't support `RETURNING` statement.

This commit ensures that mysql adapters will only consider `auto_increment` columns to be
returned after insert which also implies that only one column can be returned after insert.
  • Loading branch information
nvasilevski committed Jan 10, 2025
1 parent da32425 commit 99bd36d
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,10 @@ def supports_insert_returning?
mariadb? && database_version >= "10.5.0"
end

def return_value_after_insert?(column) # :nodoc:
supports_insert_returning? ? column.auto_populated? : column.auto_increment?
end

def get_advisory_lock(lock_name, timeout = 0) # :nodoc:
query_value("SELECT GET_LOCK(#{quote(lock_name.to_s)}, #{timeout})") == 1
end
Expand Down
17 changes: 17 additions & 0 deletions activerecord/test/cases/persistence_test.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# frozen_string_literal: true

require "cases/helper"
require "models/auto_id"
require "models/aircraft"
require "models/dashboard"
require "models/clothing_item"
Expand Down Expand Up @@ -38,6 +39,22 @@ def test_populates_non_primary_key_autoincremented_column
assert_not_nil topic.attributes["id"]
end

def test_populates_autoincremented_id_pk_regardless_of_its_position_in_columns_list
auto_populated_column_names = AutoId.columns.select(&:auto_populated?).map(&:name)

# It's important we test a scenario where tables has more than one auto populated column
# and the first column is not the primary key. Otherwise it will be a regular test not asserting this special case.
assert auto_populated_column_names.size > 1
assert_not_equal AutoId.primary_key, auto_populated_column_names.first

record = AutoId.create!
last_id = AutoId.last.id

assert_not_nil last_id
assert last_id > 0
assert_equal last_id, record.id
end

def test_populates_non_primary_key_autoincremented_column_for_a_cpk_model
order = Cpk::Order.create(shop_id: 111_222)

Expand Down
8 changes: 7 additions & 1 deletion activerecord/test/schema/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,14 @@
end

create_table :auto_id_tests, force: true, id: false do |t|
t.primary_key :auto_id
t.integer :value
default_func = if ActiveRecord::TestCase.current_adapter?(:SQLite3Adapter)
-> { "(CAST(40 AS INTEGER) + CAST(2 AS INTEGER))" }
else
-> { "(40 + 2)" }
end
t.integer :default_func_value, default: default_func
t.primary_key :auto_id
end

create_table :binaries, force: true do |t|
Expand Down

0 comments on commit 99bd36d

Please sign in to comment.