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

Simplification + solution for group concatenation exercise #289

Open
wants to merge 2 commits into
base: gh-pages
Choose a base branch
from

Conversation

colinsauze
Copy link

@colinsauze colinsauze commented Jul 24, 2019

Removes the requirement to order the results in final aggregation exercise on concatenating. There was no solution for this exercise and solving it required using sub queries which haven't been discussed until now.

I've simplified it to no need the results to be ordered, but instead to require a colon character as a separator. This way there's still a reason to use group_concat.

This has been previously discussed in bugs #158, #255 and #288. A previous solution was proposed and then retracted in pull request #210.

Please delete the text below before submitting your contribution.


Thanks for contributing! If this contribution is for instructor training, please send an email to checkout@carpentries.org with a link to this contribution so we can record your progress. You’ve completed your contribution step for instructor checkout just by submitting this contribution.

Please keep in mind that lesson maintainers are volunteers and it may be some time before they can respond to your contribution. Although not all contributions can be incorporated into the lesson materials, we appreciate your time and effort to improve the curriculum. If you have any questions about the lesson maintenance process or would like to volunteer your time as a contribution reviewer, please contact Kate Hertweck (k8hertweck@gmail.com).


Removes the requirement to order the results in final aggregation exercise on concatenating. There was no solution for this exercise and solving it required using sub queries which haven't been discussed until now. 

I've simplified it to no need the results to be ordered, but instead to require a colon character as a separator. This way there's still a reason to use group_concat. 

This has been previously discussed in bugs swcarpentry#158, swcarpentry#255 and swcarpentry#288. A previous solution was proposed and then retracted in pull request swcarpentry#210.
@remram44
Copy link
Contributor

@remram44 remram44 commented Jul 24, 2019

How about the first question asks them to list the family names separated by a comma, and then "can you find a way to list the scientists' full names?" as the add-on?

SELECT group_concat(family, ',') FROM Person;
and then
SELECT group_concat(personal || ' ' || family, ',') FROM Person;

@colinsauze
Copy link
Author

@colinsauze colinsauze commented Jul 25, 2019

I like that idea, it means they learners are doing a bit more work and digging deeper into group_concat.

@remram44
Copy link
Contributor

@remram44 remram44 commented Jul 25, 2019

Sweet! Note that there is already this line above the part you added:

Use this to produce a one-line list of scientists' names

so this should probably be the first challenge question.

Thank you so much @colinsauze, we'll finally close all those issues.

@remram44
Copy link
Contributor

@remram44 remram44 commented Jul 25, 2019

@henrykironde I no longer have write access for some reason, so you'll have to merge this when it's ready

@henrykironde
Copy link
Contributor

@henrykironde henrykironde commented Sep 18, 2019

Thanks @colinsauze for the addition, do you want to make the requested changes that @remram44 requested. we would and do really appreciate these changes

@colinsauze
Copy link
Author

@colinsauze colinsauze commented Sep 18, 2019

I thought I made them already in my post from july 25th (dd2a8dc), but they still haven't been accepted and merged.

@danmichaelo
Copy link
Contributor

@danmichaelo danmichaelo commented May 28, 2020

@henrykironde , is there a chance you could merge this? Afaics, the current pull request resolves the issue in a good way.

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

4 participants