Skip to content

Commit 1f29391

Browse files
sevenseacatsevenseacat
authored
fix: apply join_relationship sort/limit in many_to_many lateral joins (#666)
When a many_to_many relationship uses a join_relationship with sort/limit/filter, those constraints weren't being applied in the lateral join query. Two issues: 1. The sort/limit/filter from the join_relationship weren't being added to the through_query at all 2. When the through_query is wrapped in a subquery, the parent correlation needs to be inside the subquery so the limit applies per-parent
1 parent c11024b commit 1f29391

File tree

3 files changed

+263
-40
lines changed

3 files changed

+263
-40
lines changed

‎lib/data_layer.ex‎

Lines changed: 99 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1243,6 +1243,14 @@ defmodule AshPostgres.DataLayer do
12431243
})
12441244
|> Ash.Query.set_context(through_relationship.context)
12451245
|> Ash.Query.do_filter(through_relationship.filter)
1246+
|> Ash.Query.sort(through_relationship.sort)
1247+
|> then(fn q ->
1248+
if through_relationship.limit do
1249+
Ash.Query.limit(q, through_relationship.limit)
1250+
else
1251+
q
1252+
end
1253+
end)
12461254
|> Ash.Query.set_tenant(source_query.tenant)
12471255
|> set_lateral_join_prefix(query)
12481256
|> case do
@@ -1257,10 +1265,24 @@ defmodule AshPostgres.DataLayer do
12571265
{:ok, through_query} ->
12581266
through_query = Ecto.Query.exclude(through_query, :select)
12591267

1268+
# Determine if we need to wrap through_query in a subquery
1269+
needs_subquery? =
1270+
through_query.limit != nil || through_query.order_bys != [] ||
1271+
(through_query.joins && through_query.joins != []) ||
1272+
(Ash.Resource.Info.multitenancy_strategy(relationship.through) == :context &&
1273+
source_query.tenant)
1274+
12601275
through_query =
1261-
if (through_query.joins && through_query.joins != []) ||
1262-
(Ash.Resource.Info.multitenancy_strategy(relationship.through) == :context &&
1263-
source_query.tenant) do
1276+
if needs_subquery? do
1277+
# When wrapping in subquery, put parent correlation inside so that
1278+
# any limit/order_by is applied per-parent, not globally
1279+
through_query =
1280+
from(through in through_query,
1281+
where:
1282+
field(through, ^source_attribute_on_join_resource) ==
1283+
field(parent_as(^0), ^source_attribute)
1284+
)
1285+
12641286
subquery(
12651287
set_subquery_prefix(
12661288
through_query,
@@ -1274,27 +1296,46 @@ defmodule AshPostgres.DataLayer do
12741296

12751297
if query.__ash_bindings__[:__order__?] do
12761298
subquery =
1277-
subquery(
1278-
from(
1279-
destination in query,
1280-
select_merge: %{__order__: over(row_number(), :order)},
1281-
join: through in ^through_query,
1282-
as: ^through_binding,
1283-
on:
1284-
field(through, ^destination_attribute_on_join_resource) ==
1285-
field(destination, ^destination_attribute),
1286-
where:
1287-
field(through, ^source_attribute_on_join_resource) ==
1288-
field(
1289-
parent_as(^0),
1290-
^source_attribute
1291-
)
1299+
if needs_subquery? do
1300+
# Parent correlation is already in through_query subquery
1301+
subquery(
1302+
from(
1303+
destination in query,
1304+
select_merge: %{__order__: over(row_number(), :order)},
1305+
join: through in ^through_query,
1306+
as: ^through_binding,
1307+
on:
1308+
field(through, ^destination_attribute_on_join_resource) ==
1309+
field(destination, ^destination_attribute)
1310+
)
1311+
|> set_subquery_prefix(
1312+
source_query,
1313+
relationship.destination
1314+
)
12921315
)
1293-
|> set_subquery_prefix(
1294-
source_query,
1295-
relationship.destination
1316+
else
1317+
subquery(
1318+
from(
1319+
destination in query,
1320+
select_merge: %{__order__: over(row_number(), :order)},
1321+
join: through in ^through_query,
1322+
as: ^through_binding,
1323+
on:
1324+
field(through, ^destination_attribute_on_join_resource) ==
1325+
field(destination, ^destination_attribute),
1326+
where:
1327+
field(through, ^source_attribute_on_join_resource) ==
1328+
field(
1329+
parent_as(^0),
1330+
^source_attribute
1331+
)
1332+
)
1333+
|> set_subquery_prefix(
1334+
source_query,
1335+
relationship.destination
1336+
)
12961337
)
1297-
)
1338+
end
12981339

12991340
{:ok,
13001341
from(source in data_layer_query,
@@ -1308,26 +1349,44 @@ defmodule AshPostgres.DataLayer do
13081349
)}
13091350
else
13101351
subquery =
1311-
subquery(
1312-
from(
1313-
destination in query,
1314-
join: through in ^through_query,
1315-
as: ^through_binding,
1316-
on:
1317-
field(through, ^destination_attribute_on_join_resource) ==
1318-
field(destination, ^destination_attribute),
1319-
where:
1320-
field(through, ^source_attribute_on_join_resource) ==
1321-
field(
1322-
parent_as(^0),
1323-
^source_attribute
1324-
)
1352+
if needs_subquery? do
1353+
# Parent correlation is already in through_query subquery
1354+
subquery(
1355+
from(
1356+
destination in query,
1357+
join: through in ^through_query,
1358+
as: ^through_binding,
1359+
on:
1360+
field(through, ^destination_attribute_on_join_resource) ==
1361+
field(destination, ^destination_attribute)
1362+
)
1363+
|> set_subquery_prefix(
1364+
source_query,
1365+
relationship.destination
1366+
)
13251367
)
1326-
|> set_subquery_prefix(
1327-
source_query,
1328-
relationship.destination
1368+
else
1369+
subquery(
1370+
from(
1371+
destination in query,
1372+
join: through in ^through_query,
1373+
as: ^through_binding,
1374+
on:
1375+
field(through, ^destination_attribute_on_join_resource) ==
1376+
field(destination, ^destination_attribute),
1377+
where:
1378+
field(through, ^source_attribute_on_join_resource) ==
1379+
field(
1380+
parent_as(^0),
1381+
^source_attribute
1382+
)
1383+
)
1384+
|> set_subquery_prefix(
1385+
source_query,
1386+
relationship.destination
1387+
)
13291388
)
1330-
)
1389+
end
13311390

13321391
data_layer_query = Ecto.Query.exclude(data_layer_query, :distinct)
13331392

‎test/load_test.exs‎

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1027,4 +1027,143 @@ defmodule AshPostgres.Test.LoadTest do
10271027
assert %{} == author.aggregates
10281028
end
10291029
end
1030+
1031+
describe "many_to_many with join_relationship limit" do
1032+
test "many_to_many inherits limit from join_relationship" do
1033+
post =
1034+
Post
1035+
|> Ash.Changeset.for_create(:create, %{title: "test post"})
1036+
|> Ash.create!()
1037+
1038+
for order <- [3, 2, 4, 1, 5] do
1039+
user =
1040+
User
1041+
|> Ash.Changeset.for_create(:create, %{name: "User #{order}"})
1042+
|> Ash.create!()
1043+
1044+
PostFollower
1045+
|> Ash.Changeset.for_create(:create, %{
1046+
order: order,
1047+
post_id: post.id,
1048+
follower_id: user.id
1049+
})
1050+
|> Ash.create!()
1051+
1052+
user
1053+
end
1054+
1055+
# The has_many relationship - three records with the correct sort/limit
1056+
post_with_join = Ash.load!(post, :top_three_post_followers)
1057+
assert length(post_with_join.top_three_post_followers) == 3
1058+
1059+
orders = Enum.map(post_with_join.top_three_post_followers, & &1.order)
1060+
assert orders == [1, 2, 3]
1061+
1062+
# The many-to-many relationship - should use the same sort/limit
1063+
post_with_top = Ash.load!(post, :top_three_followers)
1064+
assert length(post_with_top.top_three_followers) == 3
1065+
1066+
top_user_names = post_with_top.top_three_followers |> Enum.map(& &1.name) |> Enum.sort()
1067+
assert top_user_names == ["User 1", "User 2", "User 3"]
1068+
end
1069+
1070+
test "many_to_many inherits filter, sort, and limit from join_relationship" do
1071+
post =
1072+
Post
1073+
|> Ash.Changeset.for_create(:create, %{title: "test post"})
1074+
|> Ash.create!()
1075+
1076+
for order <- [1, 2, 3, 4, 5] do
1077+
user =
1078+
User
1079+
|> Ash.Changeset.for_create(:create, %{name: "User #{order}"})
1080+
|> Ash.create!()
1081+
1082+
PostFollower
1083+
|> Ash.Changeset.for_create(:create, %{
1084+
order: order,
1085+
post_id: post.id,
1086+
follower_id: user.id
1087+
})
1088+
|> Ash.create!()
1089+
1090+
user
1091+
end
1092+
1093+
# The has_many relationship - taking the first two with order > 1
1094+
post_with_join = Ash.load!(post, :filtered_top_post_followers)
1095+
assert length(post_with_join.filtered_top_post_followers) == 2
1096+
1097+
orders = Enum.map(post_with_join.filtered_top_post_followers, & &1.order)
1098+
assert orders == [2, 3]
1099+
1100+
# The many-to-many - should apply the same limit/filter as the has_many
1101+
post_with_top = Ash.load!(post, :filtered_top_followers)
1102+
assert length(post_with_top.filtered_top_followers) == 2
1103+
1104+
top_user_names = post_with_top.filtered_top_followers |> Enum.map(& &1.name) |> Enum.sort()
1105+
assert top_user_names == ["User 2", "User 3"]
1106+
end
1107+
1108+
test "many_to_many limit is applied per-parent, not globally" do
1109+
# Create two posts
1110+
post1 =
1111+
Post
1112+
|> Ash.Changeset.for_create(:create, %{title: "post 1"})
1113+
|> Ash.create!()
1114+
1115+
post2 =
1116+
Post
1117+
|> Ash.Changeset.for_create(:create, %{title: "post 2"})
1118+
|> Ash.create!()
1119+
1120+
# Create users
1121+
users =
1122+
for i <- 1..6 do
1123+
User
1124+
|> Ash.Changeset.for_create(:create, %{name: "User #{i}"})
1125+
|> Ash.create!()
1126+
end
1127+
1128+
# Post 1 gets users 1-4 with orders 1-4
1129+
for {user, order} <- Enum.zip(Enum.take(users, 4), 1..4) do
1130+
PostFollower
1131+
|> Ash.Changeset.for_create(:create, %{
1132+
order: order,
1133+
post_id: post1.id,
1134+
follower_id: user.id
1135+
})
1136+
|> Ash.create!()
1137+
end
1138+
1139+
# Post 2 gets users 5-6 with orders 1-2
1140+
for {user, order} <- Enum.zip(Enum.drop(users, 4), 1..2) do
1141+
PostFollower
1142+
|> Ash.Changeset.for_create(:create, %{
1143+
order: order,
1144+
post_id: post2.id,
1145+
follower_id: user.id
1146+
})
1147+
|> Ash.create!()
1148+
end
1149+
1150+
# Load both posts with top_three_followers
1151+
# Without per-parent limit, post1's 4 followers would consume the global limit
1152+
# leaving post2 with fewer or none
1153+
[loaded_post1, loaded_post2] =
1154+
Post
1155+
|> Ash.Query.filter(id in [^post1.id, ^post2.id])
1156+
|> Ash.Query.sort(:title)
1157+
|> Ash.Query.load(:top_three_followers)
1158+
|> Ash.read!()
1159+
1160+
assert length(loaded_post1.top_three_followers) == 3
1161+
post1_names = loaded_post1.top_three_followers |> Enum.map(& &1.name) |> Enum.sort()
1162+
assert post1_names == ["User 1", "User 2", "User 3"]
1163+
1164+
assert length(loaded_post2.top_three_followers) == 2
1165+
post2_names = loaded_post2.top_three_followers |> Enum.map(& &1.name) |> Enum.sort()
1166+
assert post2_names == ["User 5", "User 6"]
1167+
end
1168+
end
10301169
end

‎test/support/resources/post.ex‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -851,6 +851,31 @@ defmodule AshPostgres.Test.Post do
851851

852852
has_many(:post_followers, AshPostgres.Test.PostFollower)
853853

854+
# For testing join_relationship limit inheritance
855+
has_many :top_three_post_followers, AshPostgres.Test.PostFollower do
856+
sort(order: :asc)
857+
limit(3)
858+
end
859+
860+
many_to_many(:top_three_followers, AshPostgres.Test.User,
861+
join_relationship: :top_three_post_followers,
862+
source_attribute_on_join_resource: :post_id,
863+
destination_attribute_on_join_resource: :follower_id
864+
)
865+
866+
# For testing join_relationship with filter+sort+limit inheritance
867+
has_many :filtered_top_post_followers, AshPostgres.Test.PostFollower do
868+
filter(expr(order > 1))
869+
sort(order: :asc)
870+
limit(2)
871+
end
872+
873+
many_to_many(:filtered_top_followers, AshPostgres.Test.User,
874+
join_relationship: :filtered_top_post_followers,
875+
source_attribute_on_join_resource: :post_id,
876+
destination_attribute_on_join_resource: :follower_id
877+
)
878+
854879
many_to_many(:sorted_followers, AshPostgres.Test.User,
855880
public?: true,
856881
through: AshPostgres.Test.PostFollower,

0 commit comments

Comments
 (0)