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

Support GROUP BY #158

Closed
darsana opened this issue Feb 24, 2015 · 23 comments
Closed

Support GROUP BY #158

darsana opened this issue Feb 24, 2015 · 23 comments

Comments

@darsana
Copy link

darsana commented Feb 24, 2015

I want the result for the bellow query,Can u help?

SELECT * FROM account_details WHERE UserId = '1' AND PartyRelationshipTypeId IN ( 5,6 ) GROUP BY PartyRoleID1 ORDER BY OrganizationName ASC

@begriffs
Copy link
Member

It's not yet able to do all those things. The closest you can currently do is request this:

/account_details?UserId=eq.1&order=OrganizationName.asc

The "select...in" is an upcoming feature described in issue #98. Also group by is not supported, but I'll add it to my todos.

I'll work in implementing as much as I can this weekend.

@begriffs begriffs changed the title How to make a call with IN ,GROUP BY and ORDER BY condition Support GROUP BY Feb 24, 2015
@darsana
Copy link
Author

darsana commented Feb 25, 2015

thank you for the update, We are waiting for the implementation...

@begriffs
Copy link
Member

Actually I just realized you can get much of this functionality by creating a sql view.

SELECT * FROM account_details
GROUP BY PartyRoleID1 ORDER BY OrganizationName ASC

Name it something like account_details_by_org. It will be accessible just like a table. Then access it like

/account_details_by_org?UserId=eq.1&PartyRelationshipTypeId=in.5,6

The in keyword won't work yet, but all the rest will. Like I said I'll work on implementing the in thing over the weekend.

begriffs added a commit that referenced this issue Mar 2, 2015
@razakpm
Copy link

razakpm commented Mar 2, 2015

@begriffs Thanks for the update , I'm also eagerly waiting for the IN concept. Will check the Group By and Order By concepts and will get back to you.

@darsana
Copy link
Author

darsana commented Mar 3, 2015

@begriffs Hope your working on the IN implementation in postgrest..we are eagerly waiting for the same

@begriffs
Copy link
Member

begriffs commented Mar 3, 2015

I have implemented the IN feature and it's on master now. Just adding one other feature and I'll release a new version and new binaries.

@darsana
Copy link
Author

darsana commented Mar 4, 2015

@begriffs Thanks for your update..waiting for the new release .

begriffs added a commit that referenced this issue Mar 4, 2015
@arunr307
Copy link

arunr307 commented Mar 6, 2015

Thanks for your update and new binary. Is give a solution for "IN' query. Thanks..

@begriffs
Copy link
Member

@razakpm any luck getting group by / order by to work the way you need?

@begriffs
Copy link
Member

I'm closing this issue, but we can open it again if there is anything else you need to make your group by view work.

@sscarduzio
Copy link
Contributor

I get it that you can use a view, but what's the obstacle to enriching the API with a group by? Is there some?

@ruslantalpa
Copy link
Contributor

How eould the url look like?

@ruslantalpa
Copy link
Contributor

Would*

@sscarduzio
Copy link
Contributor

sscarduzio commented Jun 4, 2016

Well for example:

GET /event_feedbacks?select=rating,count(*)&eid=not.is.null&groupby=rating
[{ "rating": 1, "count":3}, { "rating": 2, "count":8}]
SELECT rating, count(*) FROM event_feedbacks WHERE eid IS NOT NULL GROUP BY rating;

@ruslantalpa
Copy link
Contributor

The point is not expose the whole sql. Postgrest always generates a query with the same predictable shape so it's easy to optimise for it, allowing functions (like count or things like that) will allow web users to ddos any deployment with ease.

@sscarduzio
Copy link
Contributor

Wait, now I'm a bit confused, if you remember we discussed about another feature few days ago and you said it's better to delegate this kind of security checks, rate limiting and other ordinary security concerns to nginx.

Don't get me wrong, I see the point of group by being more intensive/dangerous query to be exposed directly, but I still feel this concern belongs to the same category of issues that you planned to resolve with putting an nginx in front of PostgREST: e.g. barring "groupby" as a query parameter when exposing PostgREST directly to the clients.

@ruslantalpa
Copy link
Contributor

ratelimiting is one thing able to generate a single query that can kill the server is a completely different thing, and you don't want to have a lot of configuration in nginx, just basic stuff

@sscarduzio
Copy link
Contributor

so if my logic implies group by style aggregations it's better downloading all the table in the client and crunching the numbers locally?
This might be very unpractical, what do you suggest?

@ruslantalpa
Copy link
Contributor

i am not suggesting that, you create a view/rpc for that

@Sigura
Copy link

Sigura commented Oct 4, 2016

same suggestion:

#167 (comment)

@jdgamble555
Copy link

ratelimiting is one thing able to generate a single query that can kill the server is a completely different thing, and you don't want to have a lot of configuration in nginx, just basic stuff

This is a ridiculous argument not to implement a needed feature. The database is susceptible to ddos attacks anyway as far as the limits of your infrastructure, and there are already count algorithms available on the client.

Let's add this feature please so this package can be usable without creating dozens of Views for basic use cases!

J

@mysterious-ben
Copy link

It's been quite a while since there was any update on this issue, so I wonder: is adding GROUP BY to the Postgrest API still considered dangerous?

To me, this seems to be a rather forced argument to justify not adding this very useful feature. The alternative of creating dozens of aggregated views is certainly less convenient. Regarding security: maybe it would be possible to add a config flag allowing or forbidding GROUP BY clause in the API if this is deemed to be critical?

@wolfgangwalther
Copy link
Member

Instead of digging up old, closed issues, you might look into:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

10 participants