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

Fix total entries when query has combinations #98

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

oo6
Copy link
Contributor

@oo6 oo6 commented Aug 26, 2021

Currently, Repo.paginate may throw an exception if the query has combinations.

Test case:

Post
|> Post.published()
|> union(^Post.unpublished(Post))
|> Scrivener.Ecto.Repo.paginate()

Exception message:

** (Postgrex.Error) ERROR 42601 (syntax_error) each UNION query must have the same number of columns
    query: SELECT count('*') FROM "posts" AS p0 WHERE (p0."published" = TRUE) UNION (SELECT p0."id", p0."title", p0."body", p0."published", p0."inserted_at", p0."updated_at" FROM "posts" AS p0 WHERE (p0."published" = FALSE))

Inspiration from ecto, I direct use Ecto.Query.aggregate to count total entries except query has group_bys.

@oo6
Copy link
Contributor Author

oo6 commented Jan 17, 2022

@drewolson Hi! Do you have time to review this PR? I look forward to finishing it, thanks.

@cpjolicoeur
Copy link
Member

@oo6 I know this is several years later, but we have just taken over maintenance of this repository. If you still are using scrivener and would like to get this PR included, can you please update off the latest master branch. Otherwise, we'll close this PR shortly if no activity.

@oo6
Copy link
Contributor Author

oo6 commented Sep 18, 2024

@cpjolicoeur Hey, I've updated the PR. Thank you for taking over maintenance.

@cpjolicoeur cpjolicoeur merged commit e394a05 into mojotech:master Sep 26, 2024
12 checks passed
This was referenced Sep 26, 2024
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