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

Added GROUP BY #44

Closed
wants to merge 3 commits into from
Closed

Conversation

faceleg
Copy link

@faceleg faceleg commented Jul 24, 2014

Copied methodology from your ORDER BY function.

Usage:

Model.find({
  groupBy: ['column_1', 'column_2']
}, function(models) { });

Model.find({
  groupBy: 'column_1'
}, function(models) { });

@slnode
Copy link

slnode commented Jul 24, 2014

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

@raymondfeng
Copy link
Contributor

Would you please add a test case?

@raymondfeng
Copy link
Contributor

To generalize the feature, we're considering to add more capabilities to the query filter, including:

  • groupBy/having
  • join

They should be supported by all connectors if the DB has such features.

@faceleg
Copy link
Author

faceleg commented Jul 24, 2014

@raymondfeng I've added a couple of tests.

I agree it should be generalised, but could this suffice for now? I imagine that usage would remain the same after generalisation, this way at least mysql users will be able to use groupBy.

@raymondfeng
Copy link
Contributor

The code looks good. But we cannot really benefit from it without aggregate functions, right? What do you think?

@raymondfeng
Copy link
Contributor

I think we need to first introduce aggregate operators such as count, sum, avg, distinct similar as http://docs.mongodb.org/manual/reference/sql-aggregation-comparison/.

@faceleg
Copy link
Author

faceleg commented Jul 25, 2014

I'm not sure I'm following - if one wants to GROUP BY, this works for now.

You're right that it is still lacking the others.

@raymondfeng
Copy link
Contributor

What's the purpose of GROUP BY if no aggregate functions are involved? Can you elaborate your use case?

The group by clause is used to group a list of records so that you can get aggregated results by group, for example, average score, total score, count of registered voters by county.

http://dev.mysql.com/doc/refman/5.0/en/group-by-functions.html

@faceleg
Copy link
Author

faceleg commented Jul 25, 2014

In my case, I have a list of requests, and want the first requests with a given URL, e.g.

SELECT * 
FROM requests
GROUP BY url
ORDER BY created ASC

@raymondfeng
Copy link
Contributor

You can simply use:

select * from requests order by url ASC, created ASC

@faceleg
Copy link
Author

faceleg commented Jul 26, 2014

I could, but I want only one row returned for each URL. I do not just want the sorting as your query provides - I want only one row for each URL. This is not an uncommon use of GROUP BY in my experience.

@faceleg
Copy link
Author

faceleg commented Jul 30, 2014

Any more thoughts on this PR? 🎱

@faceleg
Copy link
Author

faceleg commented Aug 11, 2014

I don't need joins for my original PR to be useful.

I now need joins if possible.

Could you briefly describe how you would want me to add the facility to do JOIN and LEFT JOIN, this way I can be more sure to implement an acceptable solution.

@raymondfeng
Copy link
Contributor

I'll add more comments after I'm back from a trip. Thanks!

@faceleg
Copy link
Author

faceleg commented Aug 29, 2014

@raymondfeng no change that you're back yet?

@raymondfeng
Copy link
Contributor

@faceleg Do you want to use the native SQL for now? For example:

ds.connector.query('SELECT DISTINCT url FROM requests GROUP BY url ORDER BY created ASC' cb);

I really prefer to add support for aggregation functions in a consistent and complete way.

@faceleg
Copy link
Author

faceleg commented Aug 30, 2014

That's a great tip, thanks - I will use that for now 👍


I was hoping that you'd be able to outline to me how you would want it implemented, and I (when I have time) could contribute said implementation in this connector. Presumably other community members would be able to do the same for the other connectors?

If there is ground work to be done in the main loopback repo, I am of course happy to contribute there as well.

I understand that you are passionate about maintaining consistency and completeness across the connectors, I share the sentiments.

@faceleg
Copy link
Author

faceleg commented Sep 9, 2014

@raymondfeng bump?

@faceleg
Copy link
Author

faceleg commented Oct 16, 2014

Sorry @raymondfeng, but bumping again

@slnode
Copy link

slnode commented Oct 16, 2014

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

@raymondfeng
Copy link
Contributor

@fabien, what's your take?

@bohrasankalp
Copy link

Is patch works on relational Model? Suppose I have 'Student' And 'Courses' through third thorough Model. I want to find Student's Courses with groupBY on particular field of courses schema. would it work??

Student.Courses({
id: $stateParams.studentId,
filter: {groupBy: 'ClassId'}
}).$promise.then(function (data) {
});

Will it work?

I checked it didn't work for relational Model.

@slnode
Copy link

slnode commented Sep 1, 2015

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

@faceleg faceleg closed this Sep 18, 2015
@faceleg
Copy link
Author

faceleg commented Sep 18, 2015

Closing, I don't use this module anymore.

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.

None yet

6 participants