Skip to content

Commit 81cbb76

Browse files
authored
Merge pull request rails#54184 from Shopify/limit-number-of-returning-columns-after-insert-to-1-when-not-supported
Fix inserts on MySQL with no `RETURNING` support for a table with multiple auto populated columns
2 parents d1d317f + ac84a0c commit 81cbb76

File tree

3 files changed

+23
-1
lines changed

3 files changed

+23
-1
lines changed

activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,10 @@ def supports_insert_returning?
174174
mariadb? && database_version >= "10.5.0"
175175
end
176176

177+
def return_value_after_insert?(column) # :nodoc:
178+
supports_insert_returning? ? column.auto_populated? : column.auto_increment?
179+
end
180+
177181
def get_advisory_lock(lock_name, timeout = 0) # :nodoc:
178182
query_value("SELECT GET_LOCK(#{quote(lock_name.to_s)}, #{timeout})") == 1
179183
end

activerecord/test/cases/persistence_test.rb

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# frozen_string_literal: true
22

33
require "cases/helper"
4+
require "models/auto_id"
45
require "models/aircraft"
56
require "models/dashboard"
67
require "models/clothing_item"
@@ -38,6 +39,22 @@ def test_populates_non_primary_key_autoincremented_column
3839
assert_not_nil topic.attributes["id"]
3940
end
4041

42+
def test_populates_autoincremented_id_pk_regardless_of_its_position_in_columns_list
43+
auto_populated_column_names = AutoId.columns.select(&:auto_populated?).map(&:name)
44+
45+
# It's important we test a scenario where tables has more than one auto populated column
46+
# and the first column is not the primary key. Otherwise it will be a regular test not asserting this special case.
47+
assert auto_populated_column_names.size > 1
48+
assert_not_equal AutoId.primary_key, auto_populated_column_names.first
49+
50+
record = AutoId.create!
51+
last_id = AutoId.last.id
52+
53+
assert_not_nil last_id
54+
assert last_id > 0
55+
assert_equal last_id, record.id
56+
end
57+
4158
def test_populates_non_primary_key_autoincremented_column_for_a_cpk_model
4259
order = Cpk::Order.create(shop_id: 111_222)
4360

activerecord/test/schema/schema.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,9 @@
105105
end
106106

107107
create_table :auto_id_tests, force: true, id: false do |t|
108-
t.primary_key :auto_id
109108
t.integer :value
109+
t.timestamp :published_at, default: -> { "CURRENT_TIMESTAMP" }
110+
t.primary_key :auto_id
110111
end
111112

112113
create_table :binaries, force: true do |t|

0 commit comments

Comments
 (0)