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

[REVIEW] Cross join feature. #5327

Merged
merged 15 commits into from
Jun 2, 2020
Merged

[REVIEW] Cross join feature. #5327

merged 15 commits into from
Jun 2, 2020

Conversation

bdice
Copy link
Contributor

@bdice bdice commented May 29, 2020

Closes #3194.

This PR adds a feature for cross join, which accepts two input tables and produces an output table with the cartesian product of their rows.

@bdice bdice self-assigned this May 29, 2020
@GPUtester
Copy link
Collaborator

Please update the changelog in order to start CI tests.

View the gpuCI docs here.

@bdice bdice marked this pull request as ready for review May 29, 2020 20:36
@bdice bdice requested review from a team as code owners May 29, 2020 20:36
@bdice bdice requested review from harrism and davidwendt May 29, 2020 20:36
@bdice bdice changed the title [WIP] Cross join feature. [REVIEW] Cross join feature. May 29, 2020
@kkraus14 kkraus14 added 3 - Ready for Review Ready for review by team 4 - Needs Review Waiting for reviewer to review or respond libcudf Affects libcudf (C++/CUDA) code. labels May 29, 2020
kkraus14
kkraus14 approved these changes May 29, 2020
View reviewed changes
Copy link
Collaborator

@kkraus14 kkraus14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cmake looks good

@davidwendt davidwendt requested a review from devavret May 29, 2020 20:50
bdice
bdice commented May 29, 2020
View reviewed changes
cpp/src/join/cross_join.cu Outdated Show resolved Hide resolved
@bdice bdice requested a review from a team as a code owner May 29, 2020 22:05
jrhemstad
jrhemstad requested changes May 29, 2020
View reviewed changes
Copy link
Contributor

@jrhemstad jrhemstad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work!

cpp/include/cudf/join.hpp Outdated Show resolved Hide resolved
cpp/src/join/cross_join.cu Show resolved Hide resolved
cpp/tests/join/cross_join_tests.cpp Show resolved Hide resolved
cpp/tests/join/cross_join_tests.cpp Outdated Show resolved Hide resolved
cwharris
cwharris approved these changes May 29, 2020
View reviewed changes
Copy link
Contributor

@cwharris cwharris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

cpp/include/cudf/join.hpp Outdated Show resolved Hide resolved
cpp/include/cudf/join.hpp Show resolved Hide resolved
cpp/include/cudf/join.hpp Outdated Show resolved Hide resolved
cpp/src/join/cross_join.cu Outdated Show resolved Hide resolved
cpp/src/join/cross_join.cu Outdated Show resolved Hide resolved
cpp/src/join/cross_join.cu Outdated Show resolved Hide resolved
codereport
codereport suggested changes May 29, 2020
View reviewed changes
cpp/include/cudf/join.hpp Outdated Show resolved Hide resolved
cpp/include/cudf/join.hpp Show resolved Hide resolved
devavret
devavret approved these changes Jun 1, 2020
View reviewed changes
Copy link
Contributor

@devavret devavret left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect. Just needs an example in the docs.

@bdice bdice requested review from jrhemstad and codereport June 1, 2020 20:51
codereport
codereport approved these changes Jun 1, 2020
View reviewed changes
Copy link
Contributor

@codereport codereport left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@harrism
Copy link
Member

harrism commented Jun 2, 2020

Perfect. Just needs an example in the docs.

If you are requesting changes please don't choose "Approve" or someone like me might drive-by merge it...

@devavret
Copy link
Contributor

devavret commented Jun 2, 2020

Perfect. Just needs an example in the docs.

If you are requesting changes please don't choose "Approve" or someone like me might drive-by merge it...

Jake already requested the changes #5327 (comment) so I was just echoing the sentiment.

@bdice bdice merged commit 9e238e7 into rapidsai:branch-0.15 Jun 2, 2020
@bdice bdice deleted the fea-cross-join branch June 2, 2020 18:58
@bdice bdice mentioned this pull request Jun 2, 2020
Merged
@wmalpica wmalpica mentioned this pull request Jun 3, 2020
Closed
@karthikeyann karthikeyann mentioned this pull request Jul 16, 2020
Merged
@GregoryKimball GregoryKimball mentioned this pull request Jul 1, 2022
Closed
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jrhemstad jrhemstad jrhemstad approved these changes

@karthikeyann karthikeyann karthikeyann left review comments

@codereport codereport codereport approved these changes

@kkraus14 kkraus14 kkraus14 approved these changes

@raydouglass raydouglass raydouglass approved these changes

@devavret devavret devavret approved these changes

@cwharris cwharris cwharris approved these changes

@harrism harrism Awaiting requested review from harrism

@davidwendt davidwendt Awaiting requested review from davidwendt

Assignees

@bdice bdice

Labels
3 - Ready for Review Ready for review by team 4 - Needs Review Waiting for reviewer to review or respond libcudf Affects libcudf (C++/CUDA) code.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEA] cross joins
10 participants
@bdice @GPUtester @harrism @devavret @cwharris @raydouglass @kkraus14 @karthikeyann @jrhemstad @codereport