diff options
Diffstat (limited to 'activerecord')
6 files changed, 30 insertions, 17 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 36bb5e1daf..e101f0919e 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -23,5 +23,13 @@ *DHH* +* Fix `#columsn_for_distinct` of MySQL and PostgreSQL to make + `ActiveRecord::FinderMethods#limited_ids_for` use correct primary key values + even if `ORDER BY` columns include other table's primary key. + + Fixes #28364. + + *Takumi Kagiyama* + Please check [5-2-stable](https://github.com/rails/rails/blob/5-2-stable/activerecord/CHANGELOG.md) for previous changes. diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index 608258d05c..fbedddd7f9 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -513,7 +513,7 @@ module ActiveRecord s.gsub(/\s+(?:ASC|DESC)\b/i, "") }.reject(&:blank?).map.with_index { |column, i| "#{column} AS alias_#{i}" } - [super, *order_columns].join(", ") + (order_columns << super).join(", ") end def strict_mode? diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 6f3db772cd..45b230f0f9 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -583,7 +583,7 @@ module ActiveRecord .gsub(/\s+NULLS\s+(?:FIRST|LAST)\b/i, "") }.reject(&:blank?).map.with_index { |column, i| "#{column} AS alias_#{i}" } - [super, *order_columns].join(", ") + (order_columns << super).join(", ") end def update_table_definition(table_name, base) # :nodoc: diff --git a/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb b/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb index d18fb97e05..0719baaa23 100644 --- a/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb +++ b/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb @@ -25,25 +25,25 @@ class Mysql2AdapterTest < ActiveRecord::Mysql2TestCase end def test_columns_for_distinct_one_order - assert_equal "posts.id, posts.created_at AS alias_0", + assert_equal "posts.created_at AS alias_0, posts.id", @conn.columns_for_distinct("posts.id", ["posts.created_at desc"]) end def test_columns_for_distinct_few_orders - assert_equal "posts.id, posts.created_at AS alias_0, posts.position AS alias_1", + assert_equal "posts.created_at AS alias_0, posts.position AS alias_1, posts.id", @conn.columns_for_distinct("posts.id", ["posts.created_at desc", "posts.position asc"]) end def test_columns_for_distinct_with_case assert_equal( - "posts.id, CASE WHEN author.is_active THEN UPPER(author.name) ELSE UPPER(author.email) END AS alias_0", + "CASE WHEN author.is_active THEN UPPER(author.name) ELSE UPPER(author.email) END AS alias_0, posts.id", @conn.columns_for_distinct("posts.id", ["CASE WHEN author.is_active THEN UPPER(author.name) ELSE UPPER(author.email) END"]) ) end def test_columns_for_distinct_blank_not_nil_orders - assert_equal "posts.id, posts.created_at AS alias_0", + assert_equal "posts.created_at AS alias_0, posts.id", @conn.columns_for_distinct("posts.id", ["posts.created_at desc", "", " "]) end @@ -52,7 +52,7 @@ class Mysql2AdapterTest < ActiveRecord::Mysql2TestCase def order.to_sql "posts.created_at desc" end - assert_equal "posts.id, posts.created_at AS alias_0", + assert_equal "posts.created_at AS alias_0, posts.id", @conn.columns_for_distinct("posts.id", [order]) end diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb index 1951230c8a..cbb6cd42b5 100644 --- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb +++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb @@ -263,25 +263,25 @@ module ActiveRecord end def test_columns_for_distinct_one_order - assert_equal "posts.id, posts.created_at AS alias_0", + assert_equal "posts.created_at AS alias_0, posts.id", @connection.columns_for_distinct("posts.id", ["posts.created_at desc"]) end def test_columns_for_distinct_few_orders - assert_equal "posts.id, posts.created_at AS alias_0, posts.position AS alias_1", + assert_equal "posts.created_at AS alias_0, posts.position AS alias_1, posts.id", @connection.columns_for_distinct("posts.id", ["posts.created_at desc", "posts.position asc"]) end def test_columns_for_distinct_with_case assert_equal( - "posts.id, CASE WHEN author.is_active THEN UPPER(author.name) ELSE UPPER(author.email) END AS alias_0", + "CASE WHEN author.is_active THEN UPPER(author.name) ELSE UPPER(author.email) END AS alias_0, posts.id", @connection.columns_for_distinct("posts.id", ["CASE WHEN author.is_active THEN UPPER(author.name) ELSE UPPER(author.email) END"]) ) end def test_columns_for_distinct_blank_not_nil_orders - assert_equal "posts.id, posts.created_at AS alias_0", + assert_equal "posts.created_at AS alias_0, posts.id", @connection.columns_for_distinct("posts.id", ["posts.created_at desc", "", " "]) end @@ -290,23 +290,23 @@ module ActiveRecord def order.to_sql "posts.created_at desc" end - assert_equal "posts.id, posts.created_at AS alias_0", + assert_equal "posts.created_at AS alias_0, posts.id", @connection.columns_for_distinct("posts.id", [order]) end def test_columns_for_distinct_with_nulls - assert_equal "posts.title, posts.updater_id AS alias_0", @connection.columns_for_distinct("posts.title", ["posts.updater_id desc nulls first"]) - assert_equal "posts.title, posts.updater_id AS alias_0", @connection.columns_for_distinct("posts.title", ["posts.updater_id desc nulls last"]) + assert_equal "posts.updater_id AS alias_0, posts.title", @connection.columns_for_distinct("posts.title", ["posts.updater_id desc nulls first"]) + assert_equal "posts.updater_id AS alias_0, posts.title", @connection.columns_for_distinct("posts.title", ["posts.updater_id desc nulls last"]) end def test_columns_for_distinct_without_order_specifiers - assert_equal "posts.title, posts.updater_id AS alias_0", + assert_equal "posts.updater_id AS alias_0, posts.title", @connection.columns_for_distinct("posts.title", ["posts.updater_id"]) - assert_equal "posts.title, posts.updater_id AS alias_0", + assert_equal "posts.updater_id AS alias_0, posts.title", @connection.columns_for_distinct("posts.title", ["posts.updater_id nulls last"]) - assert_equal "posts.title, posts.updater_id AS alias_0", + assert_equal "posts.updater_id AS alias_0, posts.title", @connection.columns_for_distinct("posts.title", ["posts.updater_id nulls first"]) end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index ebddf81449..d85910d9c6 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -1189,6 +1189,11 @@ class FinderTest < ActiveRecord::TestCase order("author_addresses_authors.id DESC").limit(3).to_a.size end + def test_find_with_eager_loading_collection_and_ordering_by_collection_primary_key + assert_equal Post.first, Post.eager_load(comments: :ratings). + order("posts.id, ratings.id, comments.id").first + end + def test_find_with_nil_inside_set_passed_for_one_attribute client_of = Company. where(client_of: [2, 1, nil], |