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

Support Sort on nested struct #3034

Merged
merged 3 commits into from
Jul 29, 2021

Conversation

sperlingxx
Copy link
Collaborator

Closes #2878

Signed-off-by: sperlingxx <lovedreamf@gmail.com>
Signed-off-by: sperlingxx <lovedreamf@gmail.com>
@sperlingxx sperlingxx requested a review from gerashegalov July 27, 2021 10:01
@sperlingxx
Copy link
Collaborator Author

build

timestamp_gen,
decimal_gen_default,
StructGen([('child1', byte_gen)]),
# string_gen,
Copy link
Collaborator

Choose a reason for hiding this comment

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

If they were no longer needed, better to remove them.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree if we don't think we need these any more then we should delete them instead of commenting them out. But that said are we removing this because of performance? If so then you could refactor it so there is a single test with one column for each type we want to test and do a single join. Was it commented out just because you were testing? It would be nice to know why these tests are being removed so we can make a proper decision on how to deal with it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These tests are necessary. I just forgot to uncomment them after the debugging work on Array of Byte. I am sorry for making you confused :(

@sameerz sameerz added the feature request New feature or request label Jul 27, 2021
@jlowe jlowe linked an issue Jul 27, 2021 that may be closed by this pull request
timestamp_gen,
decimal_gen_default,
StructGen([('child1', byte_gen)]),
# string_gen,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree if we don't think we need these any more then we should delete them instead of commenting them out. But that said are we removing this because of performance? If so then you could refactor it so there is a single test with one column for each type we want to test and do a single join. Was it commented out just because you were testing? It would be nice to know why these tests are being removed so we can make a proper decision on how to deal with it.

Signed-off-by: sperlingxx <lovedreamf@gmail.com>
@sperlingxx
Copy link
Collaborator Author

build

@sperlingxx sperlingxx merged commit 1cc4c1b into NVIDIA:branch-21.10 Jul 29, 2021
@sperlingxx sperlingxx deleted the sort_single_struct branch July 29, 2021 02:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEA] Support Sort on nested struct
5 participants