From adf09db3e9eaef38f7867a54c722e5a8bf3eeff3 Mon Sep 17 00:00:00 2001 From: Paarth Madan Date: Wed, 19 Apr 2023 15:49:13 -0400 Subject: [PATCH 1/2] CPK: Fix specifying single record in association Single records in CPK contexts will take on an array to represent their identifier. This was problematic because if only a single record is passed, it's hard to distinguish the behaviour between a list of multiple primary keys or a list representing a single primary key. As such, it's helpful to always wrap the result of the single ID in an array, so as to disambiguate the cases and create consistency in the shape of the id structure. --- .../predicate_builder/association_query_value.rb | 2 +- activerecord/test/cases/associations_test.rb | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/relation/predicate_builder/association_query_value.rb b/activerecord/lib/active_record/relation/predicate_builder/association_query_value.rb index a5a15e28836ef..ad01ff1947184 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/association_query_value.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/association_query_value.rb @@ -26,7 +26,7 @@ def ids when Array value.map { |v| convert_to_id(v) } else - convert_to_id(value) + [convert_to_id(value)] end end diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index 3c98878e0c75a..b21f48828522f 100644 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -166,6 +166,15 @@ def test_querying_by_whole_associated_records_using_query_constraints assert_equal(expected_posts.map(&:id).sort, blog_posts.map(&:id).sort) end + def test_querying_by_single_associated_record_works_using_query_constraints + comments = [sharded_comments(:great_comment_blog_post_one), sharded_comments(:great_comment_blog_post_two)] + + blog_posts = Sharded::BlogPost.where(comments: comments.last).to_a + + expected_posts = [sharded_blog_posts(:great_post_blog_two)] + assert_equal(expected_posts.map(&:id).sort, blog_posts.map(&:id).sort) + end + def test_has_many_association_with_composite_foreign_key_loads_records blog_post = sharded_blog_posts(:great_post_blog_one) From 45da2cfc908e971fc231ca9684761beb0936d6ce Mon Sep 17 00:00:00 2001 From: Paarth Madan Date: Wed, 19 Apr 2023 19:22:08 -0400 Subject: [PATCH 2/2] Eager evaluate relation in composite case Normally, it's valid syntax to pass the predicate builder a mapping from primary key to a record, to an id, or to a relation. With the composite case, there's a limitation on the types of syntax supported. Namely, the only case we support right now is mapping a tuple of columns to a tuple of corresponding values. For now, it's sufficient to extract the ids instead of evaluating the SQL at a later time. --- .../relation/predicate_builder/association_query_value.rb | 5 ++++- activerecord/test/cases/associations_test.rb | 8 ++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/relation/predicate_builder/association_query_value.rb b/activerecord/lib/active_record/relation/predicate_builder/association_query_value.rb index ad01ff1947184..7417c6a6716b8 100644 --- a/activerecord/lib/active_record/relation/predicate_builder/association_query_value.rb +++ b/activerecord/lib/active_record/relation/predicate_builder/association_query_value.rb @@ -10,7 +10,10 @@ def initialize(associated_table, value) def queries if associated_table.join_foreign_key.is_a?(Array) - ids.map { |ids_set| associated_table.join_foreign_key.zip(ids_set).to_h } + id_list = ids + id_list = id_list.pluck(primary_key) if id_list.is_a?(Relation) + + id_list.map { |ids_set| associated_table.join_foreign_key.zip(ids_set).to_h } else [ associated_table.join_foreign_key => ids ] end diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index b21f48828522f..fbfbfa456d1c4 100644 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -175,6 +175,14 @@ def test_querying_by_single_associated_record_works_using_query_constraints assert_equal(expected_posts.map(&:id).sort, blog_posts.map(&:id).sort) end + def test_querying_by_relation_with_composite_key + expected_posts = [sharded_blog_posts(:great_post_blog_one), sharded_blog_posts(:great_post_blog_two)] + + blog_posts = Sharded::BlogPost.where(comments: Sharded::Comment.where(body: "I really enjoyed the post!")).to_a + + assert_equal(expected_posts.map(&:id).sort, blog_posts.map(&:id).sort) + end + def test_has_many_association_with_composite_foreign_key_loads_records blog_post = sharded_blog_posts(:great_post_blog_one)