Skip to content

[DISCUSSION] Aggregate Queries #4197

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

Closed
dplewis opened this issue Sep 21, 2017 · 15 comments
Closed

[DISCUSSION] Aggregate Queries #4197

dplewis opened this issue Sep 21, 2017 · 15 comments

Comments

@dplewis
Copy link
Member

dplewis commented Sep 21, 2017

I wanted to start a discussion on the best way to implement Aggregate Queries into Parse Server.
Starting with the REST

curl -X GET \
    -H "X-Parse-Application-Id: ${APPLICATION_ID}" \
    -H "X-Parse-REST-API-Key: ${REST_API_KEY}" \
    -G \
    --data-urlencode 'sum=score&groupby=createdAt&groupcount=true&order=-createdAt' \
    http://localhost:1337/classes/GameScore

or

curl -X GET \
  -H "X-Parse-Application-Id: ${APPLICATION_ID}" \
  -H "X-Parse-REST-API-Key: ${REST_API_KEY}" \
  -G \
  --data-urlencode ' \
    $match: { score: 1337 }, \
    $group: { _id: null, score: { $sum: "score" } }, \
    $sort: { score: -1 }' \
  https://localhost:1337/classes/GameScore

For Postgres we could build a basic aggregate to SQL transform and let the community build it.
Similar to
https://docs.mongodb.com/manual/reference/sql-aggregation-comparison/

For the SDK we could have a query.aggregate()

Thoughts?

@flovilmart
Copy link
Contributor

If those are GET, we should probably follow the current paradigm with where=... so add a new aggregate=

/classes/MyObject?aggregate={$match...

What do you think?

@dplewis
Copy link
Member Author

dplewis commented Sep 21, 2017

Great idea that would be easier to implement then finding special $.

What are you thoughts on PG?

@flovilmart
Copy link
Contributor

I’m not expert enough with PG, but the REST interface should be simple enough to be used,

The focus should not be what is in one DB or the other, but abstracting the aggregation high enough that we can apply it to any DB provider.

@flovilmart
Copy link
Contributor

Also, we should probably restrict it to masterKey as it may spin the DB high enough to DoS it

@dplewis
Copy link
Member Author

dplewis commented Sep 21, 2017

What should we do about test cases so we can test against any DB adapter?

@flovilmart
Copy link
Contributor

The DB adapter should expose probably a ‘runAggregate’ and if available, we run the ParseRESTAggregate object into it

@dplewis
Copy link
Member Author

dplewis commented Sep 21, 2017

I can do a PR this weekend, might have to look into master key a bit. If there are any other ideas they are more than welcome.

@flovilmart
Copy link
Contributor

flovilmart commented Sep 21, 2017 via email

@flovilmart
Copy link
Contributor

What about the return types? Should not be object right? So we could basically run it through an entire new endpoint /aggregate/:className no?

@dplewis
Copy link
Member Author

dplewis commented Sep 22, 2017

I'm thinking array of results like find(). I'll have to check

@flovilmart
Copy link
Contributor

If you group by any key, the objectId won’t be relevant the contents may be ‘similar’ to an object, but the objectId may be an array no?

@dplewis
Copy link
Member Author

dplewis commented Sep 22, 2017

If we run it through a new endpoint the result will be the same. If the user is using aggregate they should handle any return values right?

@flovilmart
Copy link
Contributor

Yep, but the return value won’t be interpolated a Parse Object but as a Parse Aggregate. In anycase, I don’t think we can match an aggregate result on an object (the base of an object is objectId unicity)

@benishak
Copy link
Contributor

benishak commented Sep 22, 2017

aggregation operations are very expensive, user should be aware and informed on the impact of these operations on his DB performance. I mean we should note that in the docs.

Also we must make sure that issues like #3813 never happen, so we should have security built-in from day 1.

@srameshr
Copy link
Contributor

srameshr commented Jan 8, 2018

@flovilmart There is not even a mention of this on JS docs page. I have to dig through the API to find new methods and google what they do.

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

No branches or pull requests

4 participants