Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow select_merge in item_query (index) #929

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

krns
Copy link
Member

@krns krns commented Mar 5, 2025

Allow select_merge in item_query when live_action is :index.

Before this change you cannot use select_merge on index because it collides with the count query for the pagination and results in the following error:

the following exception happened when compiling a subquery.

    ** (Ecto.QueryError) atoms, structs, maps, lists, tuples and sources are not allowed as map values in subquery, got: `%{child_count: {:subquery, 0}}` in query:
    
    from v0 in MyApp.Parent,
      as: :parent,
      select: merge(v0, %{
      child_count:
        subquery(
          #Ecto.Query<from a0 in MyApp.Child, where: a0.parent_id == parent_as(:parent).id, select: %{result: count()}>
        )
    })
    

The subquery originated from the following query:

from v0 in subquery(from v0 in MyApp.Parent,
  as: :parent,
  select: merge(v0, %{
  child_count:
    subquery(
      #Ecto.Query<from a0 in MyApp.Child, where: a0.parent_id == parent_as(:parent).id, select: count()>
    )
})),
  select: count()

@krns
Copy link
Member Author

krns commented Mar 5, 2025

@pehbehbeh @Flo0807 this fixes the issue I told you a few weeks ago

@krns krns requested a review from pehbehbeh March 5, 2025 20:12
@Flo0807
Copy link
Collaborator

Flo0807 commented Mar 5, 2025

@pehbehbeh @Flo0807 this fixes the issue I told you a few weeks ago

👍 Looks like an easy fix, and we don't need select queries in the count query anyway, I guess.

@krns
Copy link
Member Author

krns commented Mar 5, 2025

Yes, the only needed select statement comes from the Repo.aggregate call.

@krns krns self-assigned this Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants