Ash Elixir•2y ago

Postgres Queries have wrong where clause

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) []

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
end

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
end

We could also observe this behaviour when loading a has_one relationship Versions: ash: 06329b97cf531b6b585630638028233a48a7fa0b ash_graphql: c70e7dec7dac1aac7fd40a67b51a6d61d67f9d41 ash_postgres: 9e31f905861c8f97bb2b54fd8604eb362391e675

25 Replies

barnabasjOP•2y ago

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•2y ago

🤔 so that soudns like maybe two bugs hmmmmm.... have you reproduced this in code? skipping ash_graphql I mean ohhhhhhh

barnabasjOP•2y ago

not without it no

ZachDaniel•2y ago

I think I see at least one problem, maybe one sec got one of these reproduced in test the nil attribute okay can you try ash main

barnabasjOP•2y ago

on it looks good after quick check I'm still unable to reproduce the relationship one in a test, still trying though 😉

ZachDaniel•2y ago

wait maybe I don't understand are you saying that there is still an issue?

barnabasjOP•2y ago

yes, the second problem is still there. let me describe it again: If I query the api like this
offer { id owner { name } }
offer { id owner { name } }
I get this back
{ "offer": { "id" "offer-id" "owner": null } }
{ "offer": { "id" "offer-id" "owner": null } }
but if I do

offer {
  id
  owner_id
  owner {
    name
  }
}

offer {
  id
  owner_id
  owner {
    name
  }
}

i get this back

{
  "offer": {
    "id" "offer-id"
    "owner": {
      "name": "Max"
    }
  }
}

{
  "offer": {
    "id" "offer-id"
    "owner": {
      "name": "Max"
    }
  }
}

ZachDaniel•2y ago

okay, will look into it.

barnabasjOP•2y ago

Thanks I checked the Query from the FE again and it seems that it has to do something with fragments This is the actual query that the frontend sent.

 
query Offer {
  offer {
    id
    ...Owner
  }
}

fragment Owner on Offer {
    id
    owner {
      name
    }
  }

 
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•2y ago

okay, thanks for the info i can probably fix it with that

barnabasjOP•2y ago

amazing

ZachDaniel•2y ago

hmm....actually still trying to reproduce this

    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

    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

oh there is a difference

  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

  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

That also works for me currently Is there anything special about order/owner/the relationship between them I should know about?

barnabasjOP•2y ago

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. 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. The resources are in different AshApi/Registries 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
]

[
  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•2y ago

🤔 🤔

barnabasjOP•2y ago

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

GitHub

ash_graphql/lib/graphql/resolver.ex at c70e7dec7dac1aac7fd40a67b51a...

An absinthe backed graphql API extension for the Ash Framework - ash_graphql/lib/graphql/resolver.ex at c70e7dec7dac1aac7fd40a67b51a6d61d67f9d41 · ash-project/ash_graphql

barnabasjOP•2y ago

I was now able to reproduce the issue and add a test to ash grapqhl https://github.com/ash-project/ash_graphql/pull/80

GitHub

reproduce fragment issue by barnabasJ · Pull Request #80 · ash-proj...

I was now able to reproduce the issue with the fragment. The fragment needed to be on a lower level in the query and the related resource was not allowed to be queried further up. At the top, the t...

ZachDaniel•2y ago

Amazing as always 🙂 I’ll address this today.

barnabasjOP•2y ago

Thank you

ZachDaniel•2y ago

okay, got a fix incoming Should be fixed in main 🥳

barnabasjOP•2y ago

🚀 will probably check it out tomorrow 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

** (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•2y ago

Interesting…update spark as well Ill push a fix to a new RC oh, interesting it looks like it does depend on the latest spark... oh, I bet you're doing this

calculate :foo, :bar do
  calculation ...
end

calculate :foo, :bar do
  calculation ...
end

which was actually a bug that it was supported 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

barnabasjOP•2y ago

so I just need to put the calc after the type instead of the do block right? 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•2y ago

It should work without it inside the do block now Using rc 1

Murray B•10mo ago

I am having an issue with the generated SQL when using a filter expression on a resource with a filter.

  has_many :tasks, Bordevo.Projects.Task do
      destination_attribute :id
      source_attribute :resource_id
      filter expr(:resource_type == "Task")
    end

  has_many :tasks, Bordevo.Projects.Task do
      destination_attribute :id
      source_attribute :resource_id
      filter expr(:resource_type == "Task")
    end

read :project_tasks_activity do
      argument :project_id, :uuid, allow_nil?: false
      filter expr(tasks.project_id == ^arg(:project_id))

    end

read :project_tasks_activity do
      argument :project_id, :uuid, allow_nil?: false
      filter expr(tasks.project_id == ^arg(:project_id))

    end

 FROM "public"."tasks" AS st0 WHERE (FALSE)

 FROM "public"."tasks" AS st0 WHERE (FALSE)

Does anyone know why the where (FALSE) is being generated?

ZachDaniel•10mo ago

:resource_type == "Task" :resource_type is a value, not a reference :resource_type == "Task" is always false You are looking for filter expr(resource_type == "Task")

Gaming

Programming

Postgres Queries have wrong where clause

Did you find this page helpful?