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

Fix issue when out of core sorting nested data types #2251

Merged
merged 2 commits into from
Apr 23, 2021
Merged

Fix issue when out of core sorting nested data types #2251

merged 2 commits into from
Apr 23, 2021

Conversation

revans2
Copy link
Collaborator

@revans2 revans2 commented Apr 23, 2021

This fixes #2225

@revans2
Fix issue when out of core sorting nested data types
b058bfc 
Signed-off-by: Robert (Bobby) Evans <bobby@apache.org>
@revans2 revans2 added bug Something isn't working P0 Must have for release labels Apr 23, 2021
@revans2 revans2 added this to the Apr 12 - Apr 23 milestone Apr 23, 2021
@revans2 revans2 self-assigned this Apr 23, 2021
@revans2
Copy link
Collaborator Author

revans2 commented Apr 23, 2021

build

razajafri
razajafri reviewed Apr 23, 2021
View reviewed changes
sql-plugin/src/main/scala/com/nvidia/spark/rapids/SortUtils.scala Outdated
if (hasNested) {
// so as a work around we concat all of the data together and then sort it.
// It is slower, but it works
withResource(Table.concatenate(tabs: _*)) { concated =>
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: spelling concatenate

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know what you mean. Do you want me to fully spell out "concatenate" instead of "concat" or "concatenated" instead of my shortened "concated"?

Copy link
Collaborator

@razajafri razajafri Apr 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

if you meant for this to be then I see no problem in accepting the shortened version of concatenated => concated

jlowe
jlowe previously approved these changes Apr 23, 2021
View reviewed changes
Copy link
Member

@jlowe jlowe left a comment

Choose a reason for hiding this comment

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

Just minor, non-blocking nits from me.

@revans2 revans2 mentioned this pull request Apr 23, 2021
Open
@revans2
Copy link
Collaborator Author

revans2 commented Apr 23, 2021

build

@revans2
Copy link
Collaborator Author

revans2 commented Apr 23, 2021

@jlowe @razajafri I have addressed your review comments.

@revans2 revans2 merged commit b455d1b into NVIDIA:branch-0.5 Apr 23, 2021
@revans2 revans2 deleted the merge_sort_fail branch April 23, 2021 23:16
@mythrocks mythrocks mentioned this pull request Apr 26, 2021
Closed
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
@revans2
Fix issue when out of core sorting nested data types (NVIDIA#2251)
aec126d 
Signed-off-by: Robert (Bobby) Evans <bobby@apache.org>
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
@revans2
Fix issue when out of core sorting nested data types (NVIDIA#2251)
f779834 
Signed-off-by: Robert (Bobby) Evans <bobby@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlowe jlowe jlowe approved these changes

@razajafri razajafri razajafri approved these changes

Assignees

@revans2 revans2

Labels
bug Something isn't working P0 Must have for release
Projects
None yet
Milestone
Apr 12 - Apr 23
Development

Successfully merging this pull request may close these issues.

[BUG] GpuSort fails on tables containing arrays.
3 participants
@revans2 @jlowe @razajafri