Support "unflatten" of columns flattened via `flatten_nested_columns()`: #8956

mythrocks · 2021-08-04T18:25:56Z

cudf::flatten_nested_columns() flattens out STRUCT columns into their
constituent member columns, and includes the STRUCT's validity information
as a BOOL8 column.
E.g. STRUCT_1< STRUCT_2< A, B >, C > is flattened to:
1. Null Vector for STRUCT_1
2. Null Vector for STRUCT_2
3. Member STRUCT_2::A
4. Member STRUCT_2::B
5. Member STRUCT_1::C

This commit adds an unflatten_nested_columns() method to convert back
from a flattened representation to the nested columns.

`cudf::flatten_nested_columns()` flattens out STRUCT columns into their constituent member columns, and includes the STRUCT's validity information as a BOOL8 column. E.g. STRUCT_1< STRUCT_2< A, B >, C > is flattened to: 1. Null Vector for STRUCT_1 2. Null Vector for STRUCT_2 3. Member STRUCT_2::A 4. Member STRUCT_2::B 5. Member STRUCT_1::C This commit adds an `unflatten_nested_columns()` method to convert back from a flattened representation to the nested columns.

codecov · 2021-08-04T20:38:55Z

Codecov Report

❗ No coverage uploaded for pull request base (branch-21.10@5869264). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head bdbf762 differs from pull request most recent head 94bb184. Consider uploading reports for the commit 94bb184 to get more accurate results

@@               Coverage Diff               @@
##             branch-21.10    #8956   +/-   ##
===============================================
  Coverage                ?   10.73%           
===============================================
  Files                   ?      114           
  Lines                   ?    18666           
  Branches                ?        0           
===============================================
  Hits                    ?     2003           
  Misses                  ?    16663           
  Partials                ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5869264...94bb184. Read the comment docs.

ttnghia

Just some minor comments. Overall, I would like to recommend more polishing in style/formatting. Will get back again later to see if there's anything more to suggest :)

cpp/src/structs/utilities.cpp

1. Minor reformatting. 2. num_rows init change 3. Removed TODO for blueprint null handling.

…ted-columns

mythrocks · 2021-08-10T05:13:55Z

Took @jrhemstad off the hook. Added @devavret, on whom I'll also be relying for the related structs groupby review.

cpp/src/structs/utilities.cpp

…ted-columns

mythrocks · 2021-08-13T00:31:28Z

rerun tests

mythrocks · 2021-08-13T04:06:22Z

The failure does not seem to be related to this change. From the failure logs:

18:13:51 gtests/ARROW_IO_SOURCE_TEST: error while loading shared libraries: libarrow.so.400: cannot open shared object file: No such file or directory

I'm not sure why this is happening.

ttnghia · 2021-08-13T16:34:40Z

The CI error seems to be fixed.
Rerun tests.

mythrocks · 2021-08-13T17:10:47Z

rerun tests

cpp/src/structs/utilities.cpp

…ted-columns

cpp/src/structs/utilities.cpp

cpp/tests/structs/utilities_tests.cpp

devavret

LGTM

cpp/src/structs/utilities.cpp

1. Minor formatting. 2. Reuse `is_struct()` in a couple of places. 3. Modified test numerical values.

…ted-columns

mythrocks · 2021-08-20T18:03:06Z

Thanks for the reviews, chaps. I'll check this in after CI tests pass.

mythrocks · 2021-08-20T21:28:31Z

@gpucibot merge

This commit adds support for `STRUCT` columns in `groupby`. This should now allow for `groupby` aggregations to work when any of the grouping columns are `STRUCT`, including nested `STRUCTS`. Note: `List` columns are still not supported on `groupby`, even as members of `STRUCT` columns, at any level of nesting. Only `STRUCT`, `STRUCT<STRUCT>`, etc. are currently supported. Depends on rapidsai#8956 (i.e. `unflatten_nested_columns()`).

This commit adds support for `STRUCT` columns in `groupby`. This should now allow for groupby aggregations to work when any of the grouping columns are `STRUCT`, including nested `STRUCTS`. Note: List columns are still not supported on `groupby`, even as members of `STRUCT` columns, at any level of nesting. Only `STRUCT`, `STRUCT<STRUCT>`, etc. are currently supported. Depends on #8956 (i.e. `unflatten_nested_columns()`). Authors: - MithunR (https://github.com/mythrocks) Approvers: - Jake Hemstad (https://github.com/jrhemstad) URL: #9024

mythrocks added feature request New feature or request non-breaking Non-breaking change labels Aug 4, 2021

mythrocks self-assigned this Aug 4, 2021

mythrocks requested a review from a team as a code owner August 4, 2021 18:25

mythrocks requested review from ttnghia and jrhemstad August 4, 2021 18:25

github-actions bot added CMake CMake build issue libcudf Affects libcudf (C++/CUDA) code. labels Aug 4, 2021

mythrocks added 3 - Ready for Review Ready for review by team 4 - Needs Review Waiting for reviewer to review or respond labels Aug 5, 2021

Remove mistaken change to sum_tests.

bb4dd56

ttnghia reviewed Aug 9, 2021

View reviewed changes

mythrocks added 4 commits August 9, 2021 21:56

Review comments:

e3973b8

1. Minor reformatting. 2. num_rows init change 3. Removed TODO for blueprint null handling.

Merge remote-tracking branch 'origin/branch-21.10' into unflatten-nes…

ffe69a4

…ted-columns

Changed num_rows to const.

a6a47f4

Restored num_rows calculation back to the top.

189b98e

mythrocks requested a review from devavret August 10, 2021 05:12

mythrocks removed the request for review from jrhemstad August 10, 2021 21:38

ttnghia reviewed Aug 12, 2021

View reviewed changes

cpp/src/structs/utilities.cpp Outdated Show resolved Hide resolved

ttnghia reviewed Aug 12, 2021

View reviewed changes

cpp/src/structs/utilities.cpp Outdated Show resolved Hide resolved

ttnghia reviewed Aug 12, 2021

View reviewed changes

cpp/src/structs/utilities.cpp Outdated Show resolved Hide resolved

mythrocks mentioned this pull request Aug 12, 2021

Add support for STRUCT input to groupby #9024

Merged

mythrocks added 3 commits August 12, 2021 12:20

Review: Streamline construction of vector<children>.

98da58c

Fail flattener on LIST columns.

80a5d68

Merge remote-tracking branch 'origin/branch-21.10' into unflatten-nes…

137adc6

…ted-columns

ttnghia reviewed Aug 16, 2021

View reviewed changes

cpp/src/structs/utilities.cpp Outdated Show resolved Hide resolved

mythrocks added 3 commits August 16, 2021 10:53

Moved typedefs into unnamed namespace.

2f5ab45

Merge remote-tracking branch 'origin/branch-21.10' into unflatten-nes…

692dfd0

…ted-columns

Merge remote-tracking branch 'origin/branch-21.10' into unflatten-nes…

5c777d1

…ted-columns

ttnghia reviewed Aug 17, 2021

View reviewed changes

cpp/src/structs/utilities.cpp Outdated Show resolved Hide resolved

ttnghia reviewed Aug 17, 2021

View reviewed changes

cpp/src/structs/utilities.cpp Outdated Show resolved Hide resolved

ttnghia reviewed Aug 17, 2021

View reviewed changes

cpp/src/structs/utilities.cpp Outdated Show resolved Hide resolved

ttnghia reviewed Aug 17, 2021

View reviewed changes

cpp/tests/structs/utilities_tests.cpp Outdated Show resolved Hide resolved

devavret approved these changes Aug 19, 2021

View reviewed changes

cpp/src/structs/utilities.cpp Outdated Show resolved Hide resolved

cpp/src/structs/utilities.cpp Outdated Show resolved Hide resolved

mythrocks added 2 commits August 19, 2021 15:18

Review changes:

b02833e

1. Minor formatting. 2. Reuse `is_struct()` in a couple of places. 3. Modified test numerical values.

Merge remote-tracking branch 'origin/branch-21.10' into unflatten-nes…

0a1fa1d

…ted-columns

mythrocks requested a review from ttnghia August 19, 2021 22:27

mythrocks added 2 commits August 20, 2021 10:52

Remove struct check when looking for nested lists.

7582913

Merge remote-tracking branch 'origin/branch-21.10' into unflatten-nes…

94bb184

…ted-columns

ttnghia approved these changes Aug 20, 2021

View reviewed changes

rapids-bot bot merged commit 8c92812 into rapidsai:branch-21.10 Aug 20, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support "unflatten" of columns flattened via `flatten_nested_columns()`: #8956

Support "unflatten" of columns flattened via `flatten_nested_columns()`: #8956

mythrocks commented Aug 4, 2021

codecov bot commented Aug 4, 2021 •

edited

Loading

ttnghia left a comment •

edited

Loading

mythrocks commented Aug 10, 2021 •

edited

Loading

mythrocks commented Aug 13, 2021

mythrocks commented Aug 13, 2021

ttnghia commented Aug 13, 2021 •

edited

Loading

mythrocks commented Aug 13, 2021

devavret left a comment

mythrocks commented Aug 20, 2021

mythrocks commented Aug 20, 2021

Support "unflatten" of columns flattened via flatten_nested_columns(): #8956

Support "unflatten" of columns flattened via flatten_nested_columns(): #8956

Conversation

mythrocks commented Aug 4, 2021

codecov bot commented Aug 4, 2021 • edited Loading

Codecov Report

ttnghia left a comment • edited Loading

Choose a reason for hiding this comment

mythrocks commented Aug 10, 2021 • edited Loading

mythrocks commented Aug 13, 2021

mythrocks commented Aug 13, 2021

ttnghia commented Aug 13, 2021 • edited Loading

mythrocks commented Aug 13, 2021

devavret left a comment

Choose a reason for hiding this comment

mythrocks commented Aug 20, 2021

mythrocks commented Aug 20, 2021

Support "unflatten" of columns flattened via `flatten_nested_columns()`: #8956

Support "unflatten" of columns flattened via `flatten_nested_columns()`: #8956

codecov bot commented Aug 4, 2021 •

edited

Loading

ttnghia left a comment •

edited

Loading

mythrocks commented Aug 10, 2021 •

edited

Loading

ttnghia commented Aug 13, 2021 •

edited

Loading