fix: handle various join bugs

This commit is contained in:
Zach Daniel 2022-06-29 15:08:49 -04:00
parent 53560d82ac
commit da07ed7b6c
10 changed files with 301 additions and 73 deletions

View file

@ -39,10 +39,7 @@ defmodule AshPostgres.Aggregate do
resource,
aggregate.relationship_path
)}
],
[],
nil,
true
]
) do
{:ok, new_query} ->
{:ok,
@ -69,10 +66,7 @@ defmodule AshPostgres.Aggregate do
resource,
aggregate.relationship_path
)}
],
[],
nil,
true
]
) do
{:ok, new_query} ->
{:ok,
@ -313,11 +307,7 @@ defmodule AshPostgres.Aggregate do
if aggregate.query && aggregate.query.filter do
AshPostgres.Join.join_all_relationships(
aggregate_query,
aggregate.query.filter,
nil,
[],
nil,
true
aggregate.query.filter
)
else
{:ok, aggregate_query}

View file

@ -28,8 +28,7 @@ defmodule AshPostgres.Join do
filter,
relationship_paths \\ nil,
path \\ [],
source \\ nil,
use_root_query_bindings? \\ false
source \\ nil
) do
relationship_paths =
relationship_paths ||
@ -70,24 +69,17 @@ defmodule AshPostgres.Join do
Enum.map(path, & &1.name),
current_join_type,
source,
filter,
use_root_query_bindings?
filter
) do
{:ok, joined_query} ->
joined_query_with_distinct =
if use_root_query_bindings? do
joined_query
else
add_distinct(relationship, join_type, joined_query)
end
joined_query_with_distinct = add_distinct(relationship, join_type, joined_query)
case join_all_relationships(
joined_query_with_distinct,
filter,
[{join_type, rest_rels}],
current_path,
source,
use_root_query_bindings?
source
) do
{:ok, query} ->
{:cont, {:ok, query}}
@ -150,8 +142,7 @@ defmodule AshPostgres.Join do
root_query,
ash_query,
resource,
path,
use_root_query_bindings?
path
)
{:ok, query}
@ -165,7 +156,7 @@ defmodule AshPostgres.Join do
end
end
defp do_relationship_filter(query, nil, _, _, _, _, _), do: query
defp do_relationship_filter(query, nil, _, _, _, _), do: query
defp do_relationship_filter(
query,
@ -173,8 +164,7 @@ defmodule AshPostgres.Join do
root_query,
ash_query,
resource,
path,
use_root_query_bindings?
path
) do
filter =
resource
@ -193,7 +183,7 @@ defmodule AshPostgres.Join do
true
)
{:ok, query} = join_all_relationships(query, filter, nil, [], nil, use_root_query_bindings?)
{:ok, query} = join_all_relationships(query, filter)
from(row in query, where: ^dynamic)
end
@ -311,9 +301,13 @@ defmodule AshPostgres.Join do
defp add_distinct(relationship, join_type, joined_query) do
if relationship.cardinality == :many and join_type == :left && !joined_query.distinct do
if joined_query.group_bys && joined_query.group_bys != [] do
joined_query
else
from(row in joined_query,
distinct: ^Ash.Resource.Info.primary_key(relationship.destination)
)
end
else
joined_query
end
@ -325,8 +319,7 @@ defmodule AshPostgres.Join do
path,
join_type,
source,
filter,
use_root_query_bindings?
filter
) do
case Map.get(query.__ash_bindings__.bindings, path) do
%{type: existing_join_type} when join_type != existing_join_type ->
@ -339,8 +332,7 @@ defmodule AshPostgres.Join do
path,
join_type,
source,
filter,
use_root_query_bindings?
filter
)
_ ->
@ -354,10 +346,10 @@ defmodule AshPostgres.Join do
path,
kind,
source,
filter,
use_root_query_bindings?
filter
) do
join_relationship = Ash.Resource.Info.relationship(source, relationship.join_relationship)
join_relationship =
Ash.Resource.Info.relationship(relationship.source, relationship.join_relationship)
join_path =
Enum.reverse([
@ -409,6 +401,22 @@ defmodule AshPostgres.Join do
|> AshPostgres.DataLayer.add_binding(binding_data)
end
used_calculations =
Ash.Filter.used_calculations(
filter,
relationship.destination,
full_path
)
used_aggregates =
filter
|> AshPostgres.Aggregate.used_aggregates(relationship, used_calculations, full_path)
|> Enum.map(fn aggregate ->
%{aggregate | load: aggregate.name}
end)
use_root_query_bindings? = Enum.empty?(used_aggregates)
with {:ok, relationship_through} <-
maybe_get_resource_query(
relationship.through,
@ -422,6 +430,7 @@ defmodule AshPostgres.Join do
relationship.destination,
relationship,
query,
path,
use_root_query_bindings?
) do
relationship_through =
@ -450,20 +459,6 @@ defmodule AshPostgres.Join do
end
end)
used_calculations =
Ash.Filter.used_calculations(
filter,
relationship.destination,
full_path
)
used_aggregates =
filter
|> AshPostgres.Aggregate.used_aggregates(relationship, used_calculations, full_path)
|> Enum.map(fn aggregate ->
%{aggregate | load: aggregate.name}
end)
relationship_destination
|> AshPostgres.Aggregate.add_aggregates(used_aggregates, relationship.destination)
|> case do
@ -536,8 +531,7 @@ defmodule AshPostgres.Join do
path,
kind,
source,
filter,
use_root_query_bindings?
filter
) do
full_path = path ++ [relationship.name]
initial_ash_bindings = query.__ash_bindings__
@ -553,6 +547,22 @@ defmodule AshPostgres.Join do
query = AshPostgres.DataLayer.add_binding(query, binding_data)
used_calculations =
Ash.Filter.used_calculations(
filter,
relationship.destination,
full_path
)
used_aggregates =
filter
|> AshPostgres.Aggregate.used_aggregates(relationship, used_calculations, full_path)
|> Enum.map(fn aggregate ->
%{aggregate | load: aggregate.name}
end)
use_root_query_bindings? = Enum.empty?(used_aggregates)
case maybe_get_resource_query(
relationship.destination,
relationship,
@ -585,20 +595,6 @@ defmodule AshPostgres.Join do
end
end)
used_calculations =
Ash.Filter.used_calculations(
filter,
relationship.destination,
full_path
)
used_aggregates =
filter
|> AshPostgres.Aggregate.used_aggregates(relationship, used_calculations, full_path)
|> Enum.map(fn aggregate ->
%{aggregate | load: aggregate.name}
end)
relationship_destination
|> AshPostgres.Aggregate.add_aggregates(used_aggregates, relationship.destination)
|> case do

View file

@ -98,7 +98,7 @@ defmodule AshPostgres.MixProject do
{:ecto, "~> 3.8"},
{:jason, "~> 1.0"},
{:postgrex, ">= 0.0.0"},
{:ash, ash_version("~> 1.52.0-rc.2")},
{:ash, ash_version("~> 1.52.0-rc.15")},
{:git_ops, "~> 2.4.5", only: :dev},
{:ex_doc, "~> 0.22", only: :dev, runtime: false},
{:ex_check, "~> 0.14", only: :dev},

View file

@ -1,5 +1,5 @@
%{
"ash": {:hex, :ash, "1.52.0-rc.2", "8c2d1a6e385821b5f8c3a4c2d2e415fa477c5e8d4fd0654c7526c07b2e948188", [:mix], [{:comparable, "~> 1.0", [hex: :comparable, repo: "hexpm", optional: false]}, {:decimal, "~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:earmark, "~> 1.4", [hex: :earmark, repo: "hexpm", optional: true]}, {:ecto, "~> 3.7", [hex: :ecto, repo: "hexpm", optional: false]}, {:ets, "~> 0.8.0", [hex: :ets, repo: "hexpm", optional: false]}, {:jason, ">= 1.0.0", [hex: :jason, repo: "hexpm", optional: false]}, {:nimble_options, "~> 0.3.5", [hex: :nimble_options, repo: "hexpm", optional: false]}, {:picosat_elixir, "~> 0.2", [hex: :picosat_elixir, repo: "hexpm", optional: false]}, {:sourceror, "~> 0.9", [hex: :sourceror, repo: "hexpm", optional: false]}, {:timex, ">= 3.0.0", [hex: :timex, repo: "hexpm", optional: false]}], "hexpm", "5690a5da3edc8dd62d05be05446e853e329b3812e9b592013534a847646a3c80"},
"ash": {:hex, :ash, "1.52.0-rc.15", "3d2da61ff91d2e8ee39e25f3004a02b00a3f738f3c22abc285ca94b4015b19a1", [:mix], [{:comparable, "~> 1.0", [hex: :comparable, repo: "hexpm", optional: false]}, {:decimal, "~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:earmark, "~> 1.4", [hex: :earmark, repo: "hexpm", optional: true]}, {:ecto, "~> 3.7", [hex: :ecto, repo: "hexpm", optional: false]}, {:ets, "~> 0.8.0", [hex: :ets, repo: "hexpm", optional: false]}, {:jason, ">= 1.0.0", [hex: :jason, repo: "hexpm", optional: false]}, {:nimble_options, "~> 0.3.5", [hex: :nimble_options, repo: "hexpm", optional: false]}, {:picosat_elixir, "~> 0.2", [hex: :picosat_elixir, repo: "hexpm", optional: false]}, {:sourceror, "~> 0.9", [hex: :sourceror, repo: "hexpm", optional: false]}, {:stream_data, "~> 0.5.0", [hex: :stream_data, repo: "hexpm", optional: false]}, {:timex, ">= 3.0.0", [hex: :timex, repo: "hexpm", optional: false]}], "hexpm", "34ef044f4ff2a9bd66bf08b1126e932f277c9254556463c92129ca0530377cdf"},
"bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [:mix], [], "hexpm", "7af5c7e09fe1d40f76c8e4f9dd2be7cebd83909f31fee7cd0e9eadc567da8353"},
"certifi": {:hex, :certifi, "2.9.0", "6f2a475689dd47f19fb74334859d460a2dc4e3252a3324bd2111b8f0429e7e21", [:rebar3], [], "hexpm", "266da46bdb06d6c6d35fde799bcb28d36d985d424ad7c08b5bb48f5b5cdd4641"},
"combine": {:hex, :combine, "0.10.0", "eff8224eeb56498a2af13011d142c5e7997a80c8f5b97c499f84c841032e429f", [:mix], [], "hexpm", "1b1dbc1790073076580d0d1d64e42eae2366583e7aecd455d1215b0d16f2451b"},

View file

@ -0,0 +1,174 @@
{
"attributes": [
{
"allow_nil?": false,
"default": "fragment(\"uuid_generate_v4()\")",
"generated?": false,
"primary_key?": true,
"references": null,
"size": null,
"source": "id",
"type": "uuid"
},
{
"allow_nil?": true,
"default": "nil",
"generated?": false,
"primary_key?": false,
"references": null,
"size": null,
"source": "title",
"type": "text"
},
{
"allow_nil?": true,
"default": "nil",
"generated?": false,
"primary_key?": false,
"references": null,
"size": null,
"source": "score",
"type": "bigint"
},
{
"allow_nil?": true,
"default": "nil",
"generated?": false,
"primary_key?": false,
"references": null,
"size": null,
"source": "public",
"type": "boolean"
},
{
"allow_nil?": true,
"default": "nil",
"generated?": false,
"primary_key?": false,
"references": null,
"size": null,
"source": "category",
"type": "citext"
},
{
"allow_nil?": true,
"default": "\"sponsored\"",
"generated?": false,
"primary_key?": false,
"references": null,
"size": null,
"source": "type",
"type": "text"
},
{
"allow_nil?": true,
"default": "nil",
"generated?": false,
"primary_key?": false,
"references": null,
"size": null,
"source": "price",
"type": "bigint"
},
{
"allow_nil?": true,
"default": "\"0\"",
"generated?": false,
"primary_key?": false,
"references": null,
"size": null,
"source": "decimal",
"type": "decimal"
},
{
"allow_nil?": true,
"default": "nil",
"generated?": false,
"primary_key?": false,
"references": null,
"size": null,
"source": "status",
"type": "text"
},
{
"allow_nil?": true,
"default": "nil",
"generated?": false,
"primary_key?": false,
"references": null,
"size": null,
"source": "status_enum",
"type": "status"
},
{
"allow_nil?": true,
"default": "nil",
"generated?": false,
"primary_key?": false,
"references": null,
"size": null,
"source": "point",
"type": [
"array",
"float"
]
},
{
"allow_nil?": false,
"default": "fragment(\"now()\")",
"generated?": false,
"primary_key?": false,
"references": null,
"size": null,
"source": "created_at",
"type": "utc_datetime_usec"
},
{
"allow_nil?": true,
"default": "nil",
"generated?": false,
"primary_key?": false,
"references": {
"destination_field": "id",
"destination_field_default": null,
"destination_field_generated": null,
"multitenancy": {
"attribute": null,
"global": null,
"strategy": null
},
"name": "posts_author_id_fkey",
"on_delete": null,
"on_update": null,
"schema": "public",
"table": "authors"
},
"size": null,
"source": "author_id",
"type": "uuid"
}
],
"base_filter": "type = 'sponsored'",
"check_constraints": [
{
"attribute": [
"price"
],
"base_filter": "type = 'sponsored'",
"check": "price > 0",
"name": "price_must_be_positive"
}
],
"custom_indexes": [],
"has_create_action": true,
"hash": "634AAEDB95EAD37A8A8A433FF0A1FFA9D7E241D09E147CF83D4A4DE250548EFE",
"identities": [],
"multitenancy": {
"attribute": null,
"global": null,
"strategy": null
},
"repo": "Elixir.AshPostgres.TestRepo",
"schema": null,
"table": "posts"
}

View file

@ -0,0 +1,29 @@
defmodule AshPostgres.TestRepo.Migrations.MigrateResources6 do
@moduledoc """
Updates resources based on their most recent snapshots.
This file was autogenerated with `mix ash_postgres.generate_migrations`
"""
use Ecto.Migration
def up do
alter table(:posts) do
add :author_id,
references(:authors,
column: :id,
name: "posts_author_id_fkey",
type: :uuid,
prefix: "public"
)
end
end
def down do
drop constraint(:posts, "posts_author_id_fkey")
alter table(:posts) do
remove :author_id
end
end
end

View file

@ -564,6 +564,8 @@ defmodule AshPostgres.AggregateTest do
|> Ash.Changeset.set_context(%{data_layer: %{table: "comment_ratings"}})
|> Api.create!()
Logger.configure(level: :debug)
assert %Post{count_of_popular_comments: 1} =
Post
|> Ash.Query.load([

View file

@ -73,6 +73,40 @@ defmodule AshPostgres.FilterTest do
assert [] = results
end
test "with related data two steps away that matches" do
author =
Author
|> Ash.Changeset.new(%{first_name: "match"})
|> Api.create!()
post =
Post
|> Ash.Changeset.new(%{title: "title"})
|> Ash.Changeset.replace_relationship(:author, author)
|> Api.create!()
post2 =
Post
|> Ash.Changeset.new(%{title: "title2"})
|> Ash.Changeset.replace_relationship(:linked_posts, [post])
|> Ash.Changeset.replace_relationship(:author, author)
|> Api.create!()
comment =
Comment
|> Ash.Changeset.new(%{title: "not match"})
|> Ash.Changeset.replace_relationship(:post, post)
|> Ash.Changeset.replace_relationship(:author, author)
|> Api.create!()
results =
Comment
|> Ash.Query.filter(author.posts.linked_posts.title == "title")
|> Api.read!()
assert [_] = results
end
test "with related data that does match" do
post =
Post

View file

@ -25,6 +25,7 @@ defmodule AshPostgres.Test.Author do
relationships do
has_one(:profile, AshPostgres.Test.Profile)
has_many(:posts, AshPostgres.Test.Post)
end
aggregates do

View file

@ -62,6 +62,8 @@ defmodule AshPostgres.Test.Post do
end
relationships do
belongs_to(:author, AshPostgres.Test.Author)
has_many(:comments, AshPostgres.Test.Comment, destination_field: :post_id)
has_many :popular_comments, AshPostgres.Test.Comment do