Fix #columsn_for_distinct
of MySQL and PostgreSQL
Prevent `ActiveRecord::FinderMethods#limited_ids_for` from using correct primary key values even if `ORDER BY` columns include other table's primary key. Fixes #28364.
This commit is contained in:
parent
f1878fa06e
commit
2d48268d81
@ -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.
|
||||
|
@ -513,7 +513,7 @@ def columns_for_distinct(columns, orders) # :nodoc:
|
||||
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?
|
||||
|
@ -583,7 +583,7 @@ def columns_for_distinct(columns, orders) #:nodoc:
|
||||
.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:
|
||||
|
@ -25,25 +25,25 @@ def test_columns_for_distinct_zero_orders
|
||||
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 @@ def test_columns_for_distinct_with_arel_order
|
||||
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
|
||||
|
||||
|
@ -263,25 +263,25 @@ def test_columns_for_distinct_zero_orders
|
||||
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 @@ def test_columns_for_distinct_with_arel_order
|
||||
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
|
||||
|
||||
|
@ -1189,6 +1189,11 @@ def test_find_with_order_on_included_associations_with_construct_finder_sql_for_
|
||||
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],
|
||||
|
Loading…
Reference in New Issue
Block a user