Postgres Queries have wrong where clause
barnabasj:
Recently we some read actions do not return the correct data, the Queries that are logged have an obviously wrong where clause
SELECT c0."id", c0."first_name", c0."last_name", c0."title", c0."legal_entity_name", c0."is_legal_entity" FROM "contact" AS c0 WHERE (false) []
The clause should be
c0."id" = ? with the id in the parameter list
This is the resource and we query the current_user graphql query
defmodule Demo.Auth.Resources.User do
@moduledoc """
The Logged in User
"""
use Demo.AuditedResource,
data_layer: AshPostgres.DataLayer,
extensions: [AshGraphql.Resource]
attributes do
uuid_primary_key :id, generated?: false
attribute :email, :string
attribute :email_confirmed_at, :utc_datetime
attribute :contact_id, :uuid
attribute :features, Demo.Auth.Resources.User.Features
# attribute :roles, {:array, :atom}
create_timestamp :inserted_at
update_timestamp :updated_at
end
rbac do
bypass(:admin)
role(:user, [:id, :email, :email_confirmed_at, :contact, :contact_id, :features])
end
audit do
actors?(false)
timestamps?(false)
end
graphql do
type :user
queries do
read_one :user, :current_user
end
end
relationships do
belongs_to :contact, Demo.CustomerService.Resources.Contact do
filterable? false
api Demo.CustomerService.Api
end
end
actions do
defaults [:read, :create, :update]
read :current_user do
get? true
end
end
policies do
policy always() do
forbid_unless(actor_present())
authorize_if(expr(id == type(^actor(:id), Ash.Type.UUID)))
forbid_if(always())
end
end
postgres do
repo Demo.Repo
table "user"
end
endWe could also observe this behaviour when loading a has_one relationship
Versions:
ash: 06329b97cf531b6b585630638028233a48a7fa0b ash_graphql: c70e7dec7dac1aac7fd40a67b51a6d61d67f9d41 ash_postgres: 9e31f905861c8f97bb2b54fd8604eb362391e675
barnabasj:
In the case of the user we found that there is a problem with the embedded resource, if the attribute features is NULL in postgres the api returns null as the user, if there is a value in the column it returns the user. We can also get the user with the null in features if we exclude it in the graphql Query.
On the relationship side, we only get the relationship if we have also included the foreign key in the graphql query.
zachdaniel:
š¤ so that soudns like maybe two bugs
zachdaniel:
hmmmmmā¦.
zachdaniel:
have you reproduced this in code?
zachdaniel:
skipping
ash_graphql I mean
zachdaniel:
ohhhhhhh
barnabasj:
not without it no
zachdaniel:
I think I see at least one problem, maybe
zachdaniel:
one sec
zachdaniel:
got one of these reproduced in test
zachdaniel:
the nil attribute
zachdaniel:
okay can you try ash
main
barnabasj:
on it
barnabasj:
looks good after quick check
barnabasj:
Iām still unable to reproduce the relationship one in a test, still trying though š
zachdaniel:
wait
zachdaniel:
maybe I donāt understand
zachdaniel:
are you saying that there is still an issue?
barnabasj:
yes, the second problem is still there.
let me describe it again:
~~If I query the api like this
offer {
id
owner {
name
}
}I get this back
{
"offer": {
"id" "offer-id"
"owner": null
}
}but if I do
offer {
id
owner_id
owner {
name
}
}i get this back
{
"offer": {
"id" "offer-id"
"owner": {
"name": "Max"
}
}
}
zachdaniel:
okay, will look into it.
barnabasj:
Thanks
barnabasj:
I checked the Query from the FE again and it seems that it has to do something with fragments
barnabasj:
This is the actual query that the frontend sent.
query Offer {
offer {
id
...Owner
}
}
fragment Owner on Offer {
id
owner {
name
}
}with the fragment inlined it works
or adding the
ownerId in the fragment also works
zachdaniel:
okay, thanks for the info
zachdaniel:
i can probably fix it with that
barnabasj:
amazing
zachdaniel:
hmmā¦.actually still trying to reproduce this
zachdaniel:
resp =
"""
query Comments {
listComments {
...CommentPost
}
}
fragment CommentPost on Comment {
post {
text
}
}
"""
|> Absinthe.run(AshGraphql.Test.Schema)
assert {:ok, result} = resp
refute Map.has_key?(result, :errors)
post_id = post.id
assert %{data: %{"listComments" => [%{"post" => %{"text" => "foo"}}]}} = result
zachdaniel:
oh there is a difference
zachdaniel:
test "reading relationships works with fragments, without selecting the id field" do
post =
AshGraphql.Test.Post
|> Ash.Changeset.for_create(:create, text: "foo", published: true)
|> AshGraphql.Test.Api.create!()
AshGraphql.Test.Comment
|> Ash.Changeset.for_create(:create, %{text: "stuff"})
|> Ash.Changeset.force_change_attribute(:post_id, post.id)
|> AshGraphql.Test.Api.create!()
resp =
"""
query PostLibrary($published: Boolean) {
postLibrary(published: $published) {
text
...PostFragment
}
}
fragment PostFragment on Post {
comments{
text
}
}
"""
|> Absinthe.run(AshGraphql.Test.Schema,
variables: %{
"published" => true
}
)
assert {:ok, result} = resp
refute Map.has_key?(result, :errors)
assert %{data: %{"postLibrary" => [%{"text" => "foo", "comments" => [%{"text" => "stuff"}]}]}} =
result
end
zachdaniel:
That also works for me currently
zachdaniel:
Is there anything special about order/owner/the relationship between them I should know about?
barnabasj:
Not sure, I checked a couple other fragments we have in the fe, we do have other fragments with relationships that work. But the other ones i saw were all has_many relationships, this one was a has_one. I tried switching has_one with belongs_to because i thought I might have it configured wrong, but that didnāt change anything.
I thought that maybe it would only trigger the error with the Postgres DataLayer because I assumed that the ETS DataLayer might always return all the fields instead of only the selected ones. So i tried using AshPostgres in the test from AshGraphql and add the fragment but I was unable to reproduce there too.
barnabasj:
I also had the relationship declared only on the first resource offer -> owner, I then added the reverse declartion too, but I that didnāt change anything either.
barnabasj:
The resources are in different AshApi/Registries
barnabasj:
I inspected the query https://github.com/ash-project/ash_postgres/blob/9e31f905861c8f97bb2b54fd8604eb362391e675/lib/data_layer.ex#L606 and in the were clause you can see that there is no Id in the array. But I didnāt quite figure out how ash got there^^
[
query: #Ecto.Query<from e0 in Demo.HumanResources.Resources.Employee,
as: 0,
where: type(as(0).id, {:parameterized, Ash.Type.StringWrapper.EctoType, []}) in [],
select: merge(struct(e0, [:id, :email, :name, :metadata, :gender]), %{
phone:
type(
type(
fragment("(? #>> ?)", type(as(0).metadata, {:parameterized, Ash.Type.Map.EctoType, []}), [
"phone"
]),
{:parameterized, Ash.Type.StringWrapper.EctoType, allow_empty?: false, trim?: true}
),
{:parameterized, Ash.Type.StringWrapper.EctoType, allow_empty?: false, trim?: true}
),
picture:
type(
fragment(
"ash_elixir_or(?, ?)",
type(
fragment(
"(? #>> ?)",
type(as(0).metadata, {:parameterized, Ash.Type.Map.EctoType, []}),
["picture"]
),
{:parameterized, Ash.Type.StringWrapper.EctoType, allow_empty?: false, trim?: true}
),
type(as(0).picture_fallback, {:parameterized, Ash.Type.StringWrapper.EctoType, []})
),
{:parameterized, Ash.Type.StringWrapper.EctoType, allow_empty?: false, trim?: true}
)
})>,
resource: Demo.HumanResources.Resources.Employee
]
zachdaniel:
š¤ š¤
barnabasj:
So inside AshGrapqhl https://github.com/ash-project/ash_graphql/blob/c70e7dec7dac1aac7fd40a67b51a6d61d67f9d41/lib/graphql/resolver.ex#L1901
we get the fragment name as identifier.
field_or_relationship: [resource: JdlEngine.Offer.Resources.Offer, identifier: āOwnerā] field_or_relationship: nil
barnabasj:
I was now able to reproduce the issue and add a test to ash grapqhl https://github.com/ash-project/ash_graphql/pull/80
zachdaniel:
Amazing as always š
zachdaniel:
Iāll address this today.
barnabasj:
Thank you
zachdaniel:
okay, got a fix incoming
zachdaniel:
Should be fixed in main š„³
barnabasj:
š will probably check it out tomorrow
barnabasj:
Queries look good on our end too.
During testing I also tried updating ash to the latest commit on main, but Iām getting compile errors
** (CompileError) lib/demo/inventory/resources/search.ex:78: undefined function calculation/1 (there is no such import)
(ash 2.11.0-rc.0) expanding macro: Ash.Resource.Dsl.Calculations.Calculate.calculate/3
zachdaniel:
Interestingā¦update spark as well
zachdaniel:
Ill push a fix to a new RC
zachdaniel:
oh, interesting
zachdaniel:
it looks like it does depend on the latest sparkā¦
zachdaniel:
oh, I bet youāre doing this
calculate :foo, :bar do
calculation ...
end
zachdaniel:
which was actually a bug that it was supported
zachdaniel:
weāve actually kinda figured that out, realized we can support it easily. Iāll push rc.1 that supports that syntax and then you can upgrade
barnabasj:
so I just need to put the calc after the type instead of the do block right?
barnabasj:
after you wrote this i remember i had a similar problem once, where i had the option specified at the top and inside the do block and I was wondering why it was not working because the one in the do block overwrote the outer one.
zachdaniel:
It should work without it inside the do block now
zachdaniel:
Using rc 1