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.

Sign up for GitHub

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

Jump to bottom

Allow passing :* as a column name in the Repo.aggregate/{3,4} #3175

Closed
hauleth opened this issue Dec 1, 2019 · 3 comments · Fixed by #3176
Closed

Allow passing :* as a column name in the Repo.aggregate/{3,4} #3175

hauleth opened this issue Dec 1, 2019 · 3 comments · Fixed by #3176

Comments

@hauleth
Copy link
Contributor

hauleth commented Dec 1, 2019

Reasoning the same as in #2595.
@josevalim
Copy link
Member

Cool, PR is welcome, please! We can use a fragment to do count(fragment("*")).

hauleth added a commit to hauleth/ecto that referenced this issue Dec 2, 2019
@hauleth
feat(Ecto.Repo): allow aggregating over *
37f03bc 
This is done to provide similar API to `Ecto.Query.API.count/0` as
counting over `*` is faster in some DB than counting over column, even
when column is `NOT NULL` and indexed.

Close elixir-ecto#3175
@hauleth hauleth mentioned this issue Dec 2, 2019
Merged
josevalim pushed a commit that referenced this issue Dec 2, 2019
@hauleth @josevalim
Allow aggregating over * (#3176)
70a9294 
This is done to provide similar API to `Ecto.Query.API.count/0` as
counting over `*` is faster in some DB than counting over column, even
when column is `NOT NULL` and indexed.

Close #3175
@michalmuskala
Copy link
Member

What do you think about providing aggregare/2 for this instead? I think this would mirror the count/0 API better

@hauleth
Copy link
Contributor Author

hauleth commented Dec 2, 2019

@michalmuskala I didn't do so to provide a way to pass an options to the call as for me it would be weird to have Repo.aggregate/3 to work differently depending on 3rd argument.

josevalim pushed a commit that referenced this issue Dec 2, 2019
Introduce Repo.aggregate/2 instead, see #3175
a840b6c
sthagen added a commit to sthagen/elixir-ecto-ecto that referenced this issue Dec 2, 2019
@sthagen
Merge pull request #4 from elixir-ecto/master
4e80b62 
Introduce Repo.aggregate/2 instead, see elixir-ecto#3175
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Allow aggregating over * hauleth/ecto
3 participants
@josevalim @hauleth @michalmuskala