Skip to content

Commit c2c861f

Browse files
committed
Always request primary_key in RETURNING if no other columns requested
Prior 7.1 Rails always included `primary_key` in `RETURNING` clause on record creation. This was changed in 7.1 to include more auto-populated columns if such columns exist. This change lead to situations where no columns were requested in `RETURNING` clause, even the `primary_key`. This change brings back the old behavior of always requesting the `primary_key` in `RETURNING` clause if no other columns are requested.
1 parent 25b7f64 commit c2c861f

File tree

5 files changed

+51
-2
lines changed

5 files changed

+51
-2
lines changed

activerecord/lib/active_record/model_schema.rb

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -431,8 +431,12 @@ def columns
431431
end
432432

433433
def _returning_columns_for_insert # :nodoc:
434-
@_returning_columns_for_insert ||= columns.filter_map do |c|
435-
c.name if connection.return_value_after_insert?(c)
434+
@_returning_columns_for_insert ||= begin
435+
auto_populated_columns = columns.filter_map do |c|
436+
c.name if connection.return_value_after_insert?(c)
437+
end
438+
439+
auto_populated_columns.empty? ? Array(primary_key) : auto_populated_columns
436440
end
437441
end
438442

activerecord/test/cases/persistence_test.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
require "models/chat_message"
2626
require "models/default"
2727
require "models/post_with_prefetched_pk"
28+
require "models/pk_autopopulated_by_a_trigger_record"
2829

2930
class PersistenceTest < ActiveRecord::TestCase
3031
fixtures :topics, :companies, :developers, :accounts, :minimalistics, :authors, :author_addresses,
@@ -1574,6 +1575,13 @@ def test_it_is_possible_to_update_parts_of_the_query_constraints_config
15741575

15751576
assert_equal("blue", ClothingItem.find_by(id: clothing_item.id).color)
15761577
end
1578+
1579+
def test_model_with_no_auto_populated_fields_still_returns_primary_key_after_insert
1580+
record = PkAutopopulatedByATriggerRecord.create
1581+
1582+
assert_not_nil record.id
1583+
assert record.id > 0
1584+
end if current_adapter?(:PostgreSQLAdapter)
15771585
end
15781586

15791587
class QueryConstraintsTest < ActiveRecord::TestCase

activerecord/test/cases/view_test.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,12 @@ def test_insert_record
193193
assert_equal "Rails in Action", new_book.name
194194
end
195195

196+
def test_insert_record_populates_primary_key
197+
book = PrintedBook.create! name: "Rails in Action", status: 0, format: "paperback"
198+
assert_not_nil book.id
199+
assert book.id > 0
200+
end if current_adapter?(:PostgreSQLAdapter, :SQLite3Adapter) && supports_insert_returning?
201+
196202
def test_update_record_to_fail_view_conditions
197203
book = PrintedBook.first
198204
book.format = "ebook"
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
# frozen_string_literal: true
2+
3+
class PkAutopopulatedByATriggerRecord < ActiveRecord::Base
4+
self.primary_key = :id
5+
end

activerecord/test/schema/schema.rb

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1483,6 +1483,32 @@
14831483
end
14841484
end
14851485

1486+
if ActiveRecord::TestCase.current_adapter?(:PostgreSQLAdapter)
1487+
ActiveRecord::Base.connection.create_table :pk_autopopulated_by_a_trigger_records, force: true, id: false do |t|
1488+
t.integer :id, null: false
1489+
end
1490+
1491+
ActiveRecord::Base.connection.execute(
1492+
<<-SQL
1493+
CREATE OR REPLACE FUNCTION populate_column()
1494+
RETURNS TRIGGER AS $$
1495+
DECLARE
1496+
max_value INTEGER;
1497+
BEGIN
1498+
SELECT MAX(id) INTO max_value FROM pk_autopopulated_by_a_trigger_records;
1499+
NEW.id = COALESCE(max_value, 0) + 1;
1500+
RETURN NEW;
1501+
END;
1502+
$$ LANGUAGE plpgsql;
1503+
1504+
CREATE TRIGGER before_insert_trigger
1505+
BEFORE INSERT ON pk_autopopulated_by_a_trigger_records
1506+
FOR EACH ROW
1507+
EXECUTE FUNCTION populate_column();
1508+
SQL
1509+
)
1510+
end
1511+
14861512
Course.connection.create_table :courses, force: true do |t|
14871513
t.column :name, :string, null: false
14881514
t.column :college_id, :integer, index: true

0 commit comments

Comments
 (0)