Add support for struct type in ORC writer #9025

vuule · 2021-08-12T08:16:48Z

Fixes #7830, #8443

Features:

Use the new table metadata type that matches the table hierarchy, table_input_metadata.
Support struct columns in the writer.

Changes:

Null masks are encoded as aligned rowgroups to avoid invalid bits when the number of encoded rows is not divisible by 8 (except for the last rowgroup in each stripe). This also affects list columns. The issue is equivalent to [BUG] Bools written by cuIO ORC writer don't match when read by pyarrow/pyorc #6763 (boolean columns only).
Added pushdown masks that are used to determine which child elements should not be encoded, including null mask bits.
Use pushdown masks for rowgroup alignment, null mask encoding and value encoding.
Separated the null mask encoding from value encoding - can be further moved to a separate kernel call.

Breaking because the table metadata type has changed.

… struct columns

This reverts commit 6940680.

…fea-orc-writer-struct

…fea-orc-write-struct

vyasr

Python/Cython approval, with a couple of small questions.

C++/CUDA looked OK, but probably best to get approval from someone with greater familiarity. I didn't review those files in depth.

Is there a reason that submodule updates are present in this PR? The current build failure may be related since it looked like it was failing to pull in jitify.

vyasr · 2021-09-17T16:00:15Z

python/cudf/cudf/_lib/orc.pyx

+            table_view_from_table(table, ignore_index=True)
+        )
+        if self.index is not False:
+            if isinstance(table._index, cudf.core.multiindex.MultiIndex):


In write_orc the corresponding block checks for any non-RangeIndex type. Is that discrepancy intentional or meaningful? Or do you always get either a MultiIndex or a RangeIndex?

I have to defer to @devavret or @galipremsagar on this one. The Python changes in this PR are based on the equivalent Parquet PR #7461.

This code block is a replacement of the logic of utils.get_column_names which assigns the names of the columns to metadata. We don't want to update the metadata for RangeIndex because metadata contains only information on libcudf column_views. We could get a single series materialized index too, which is captured by the second part of the branch.

Regarding writing RangeIndex, this is currently broken #7011 (and re-reported in #9216) because it might take some smarts to merge range indices across different calls to write_table. The RangeIndex only gets written in the pandas specific metadata and passes straight through libcudf untouched. If you have some time, I'd invite you to take on #7011

python/cudf/cudf/tests/test_orc.py

…fea-orc-write-struct

vuule · 2021-09-17T18:03:10Z

Is there a reason that submodule updates are present in this PR? The current build failure may be related since it looked like it was failing to pull in jitify.

They got pulled in when I merged 21.10. Not sure what I did wrong and how to revert it. I have these two files marked as modified in git. When I try to update submodules, it fails with No url found for submodule path 'thirdparty/jitify' in .gitmodules.

vuule · 2021-09-17T19:44:15Z

rerun tests

devavret

Partial review but GitHub won't let me reply to @Vyas' comment without publishing my review. 🤷‍♂️

cpp/include/cudf/io/types.hpp

cpp/src/io/orc/stripe_init.cu

devavret · 2021-09-17T20:32:05Z

python/cudf/cudf/_lib/orc.pyx

+            table_view_from_table(table, ignore_index=True)
+        )
+        if self.index is not False:
+            if isinstance(table._index, cudf.core.multiindex.MultiIndex):


This code block is a replacement of the logic of utils.get_column_names which assigns the names of the columns to metadata. We don't want to update the metadata for RangeIndex because metadata contains only information on libcudf column_views. We could get a single series materialized index too, which is captured by the second part of the branch.

Regarding writing RangeIndex, this is currently broken #7011 (and re-reported in #9216) because it might take some smarts to merge range indices across different calls to write_table. The RangeIndex only gets written in the pandas specific metadata and passes straight through libcudf untouched. If you have some time, I'd invite you to take on #7011

devavret

LGTM

cpp/src/io/orc/writer_impl.cu

…fea-orc-write-struct

rgsl888prabhu

Initial pass, still going through PR.

cpp/src/io/orc/stripe_enc.cu

vuule · 2021-09-22T22:31:40Z

@gpucibot merge

This reverts commit 2c6b39b.

vuule added 16 commits July 28, 2021 13:22

enable struct columns on the python layer; enable stream creation for…

b28b2b9

… struct columns

move input metadata types to types.hpp

77e07eb

replace table_metadata and table_metadata_with_nullability; update tests

d983db4

simplify table creation in orc tests

524f7db

metadata

295312a

chunked metadata test fix; nullable getter fix

f6dba5b

add list column to a metadata test

6940680

Revert "add list column to a metadata test"

08bf157

This reverts commit 6940680.

refactor regular and chunked orc writer options

3abf86d

add the metadata + list test again

c06fcbc

generate default column names

eec7941

subtype footer

7c74d95

Merge branch 'branch-21.10' of https://github.com/rapidsai/cudf into …

03bd98f

…fea-orc-writer-struct

Merge branch 'branch-21.10' of https://github.com/rapidsai/cudf into …

94d9892

…fea-orc-writer-struct

Merge branch 'branch-21.10' of https://github.com/rapidsai/cudf into …

6e83117

…fea-orc-writer-struct

Merge branch 'branch-21.10' of https://github.com/rapidsai/cudf into …

c90d04a

…fea-orc-writer-struct

vuule self-assigned this Aug 12, 2021

github-actions bot added Python Affects Python cuDF API. libcudf Affects libcudf (C++/CUDA) code. labels Aug 12, 2021

don't use valid_buf as null_mask; remove a redundant syncthreads

0fa6aa7

vuule added 9 commits August 17, 2021 13:44

typo fix

f9cec92

Merge branch 'branch-21.10' of https://github.com/rapidsai/cudf into …

8e333c9

…fea-orc-write-struct

Merge branch 'branch-21.10' of https://github.com/rapidsai/cudf into …

4a65fad

…fea-orc-write-struct

Merge branch 'branch-21.10' of https://github.com/rapidsai/cudf into …

184c686

…fea-orc-write-struct

pushdown null masks pt 1

444d4cf

pushdown null masks pt2

df2e916

pushdown mask different approach

6e76ccc

pushdown for struct - host side copmplete

7682899

Merge branch 'branch-21.10' of https://github.com/rapidsai/cudf into …

11eef31

…fea-orc-write-struct

vuule added 5 commits September 15, 2021 15:17

Merge branch 'branch-21.10' of https://github.com/rapidsai/cudf into …

db2a8cc

…fea-orc-write-struct

Merge branch 'branch-21.10' of https://github.com/rapidsai/cudf into …

f05a503

…fea-orc-write-struct

rename bit_is_set_or

b9ef40a

have orc_column_device_view inherit from column_device_view

cacef12

sliced table support

b9214c4

vyasr approved these changes Sep 17, 2021

View reviewed changes

Merge branch 'branch-21.10' of https://github.com/rapidsai/cudf into …

3623391

…fea-orc-write-struct

delete submodules (?)

be18ba4

devavret reviewed Sep 17, 2021

View reviewed changes

devavret approved these changes Sep 17, 2021

View reviewed changes

cpp/src/io/orc/writer_impl.cu Outdated Show resolved Hide resolved

vuule added 5 commits September 17, 2021 17:21

enable multi-rowgroup borrow even without pushdown masks

3ddd197

review changes mostly

181f897

experimental

90bdbb5

list_struct_buff as module fixture

f681fdb

Merge branch 'branch-21.10' of https://github.com/rapidsai/cudf into …

8c20fb9

…fea-orc-write-struct

rgsl888prabhu reviewed Sep 20, 2021

View reviewed changes

cpp/src/io/orc/stripe_enc.cu Show resolved Hide resolved

cpp/src/io/orc/stripe_enc.cu Show resolved Hide resolved

vuule added 4 - Needs cuIO Reviewer and removed 2 - In Progress Currently a work in progress labels Sep 21, 2021

rgsl888prabhu approved these changes Sep 22, 2021

View reviewed changes

rapids-bot bot merged commit 2c6b39b into rapidsai:branch-21.10 Sep 22, 2021

vuule deleted the fea-orc-write-struct branch September 22, 2021 22:31

devavret added a commit to devavret/cudf that referenced this pull request Sep 23, 2021

Revert "Add support for struct type in ORC writer (rapidsai#9025)"

727a9c2

This reverts commit 2c6b39b.

vuule mentioned this pull request Sep 23, 2021

[FEA] Support an incoming schema for ORC writing #8443

Closed

vyasr added 4 - Needs Review Waiting for reviewer to review or respond and removed 4 - Needs cuIO Reviewer labels Feb 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for struct type in ORC writer #9025

Add support for struct type in ORC writer #9025

vuule commented Aug 12, 2021 •

edited

Loading

vyasr left a comment •

edited

Loading

vyasr Sep 17, 2021

vuule Sep 17, 2021

devavret Sep 17, 2021

vuule commented Sep 17, 2021

vuule commented Sep 17, 2021

devavret left a comment

devavret Sep 17, 2021

devavret left a comment

rgsl888prabhu left a comment

vuule commented Sep 22, 2021

Add support for struct type in ORC writer #9025

Add support for struct type in ORC writer #9025

Conversation

vuule commented Aug 12, 2021 • edited Loading

vyasr left a comment • edited Loading

Choose a reason for hiding this comment

vyasr Sep 17, 2021

Choose a reason for hiding this comment

vuule Sep 17, 2021

Choose a reason for hiding this comment

devavret Sep 17, 2021

Choose a reason for hiding this comment

vuule commented Sep 17, 2021

vuule commented Sep 17, 2021

devavret left a comment

Choose a reason for hiding this comment

devavret Sep 17, 2021

Choose a reason for hiding this comment

devavret left a comment

Choose a reason for hiding this comment

rgsl888prabhu left a comment

Choose a reason for hiding this comment

vuule commented Sep 22, 2021

vuule commented Aug 12, 2021 •

edited

Loading

vyasr left a comment •

edited

Loading