-
Notifications
You must be signed in to change notification settings - Fork 915
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
[FEA] Nulls pushdowns for LIST columns #10291
Comments
For posterity, can you summarize how a non-empty null list breaks things? I liked your naming of "sanitize" as opposed to "superimpose" or "pushdown" in this scenario. So something like Or maybe "normalize"? |
I think the easiest way to implement this is to simply perform an identity gather (i.e., gather map Unfortunately, I spoke with @nvdbaranec and In addition, I'd like to see two additional APIs for determining if sanitization is necessary:
|
We also have a decision to make about who is responsible for list sanitization. As I see it, we only have two options:
(1.) is obviously running with scissors, but it is also far more performant and less memory intensive. This is especially relevant when using the same list column in multiple operations where you can sanitize it once and reuse it multiple times. (2.) is safer, more convenient, but very expensive. In the case of reusing the same list column many times, we'd have to sanitize it every time, incurring a deep copy and the compute overhead of the sanitization. Personally, in the spirit of "not making decisions" I would opt for (1.). First and foremost, libcudf is a performance oriented library and there are higher level layers to add convenience. To ease the burden, we would provide APIs for checking if sanitization is needed (described above): a.) An approximate/fast one that only checks the null counts of all the nested list columns Additionally, I think we can guarantee that libcudf will never return a list column that needs sanitization (including file readers) so long as we are never given a dirty list column as input. (I assume if a file contains a dirty list column, we would sanitize it as part of reading it and returning it. @devavret @vuule correct me if I'm wrong). So the only time a list column would need sanitization is if it's coming from somewhere other than libcudf. Conceivably, we could put sanitization logic in interop APIs like That way, the only way to get a dirty column is to manually construct a |
I think this is a pretty safe assumption (though we'd certainly have to retrofit some old stuff). It's pretty common for list-based functions to rely on gather() for final assembly so if we fix that, we're 80% of the way there. |
This is always true for parquet, because parquet has a different representation which we convert from and i don't think we can create a dirty list from it, ever. @nvdbaranec, right? |
I'm pretty sure ORC by definition cannot have elements in a null list. Content of null elements is never encoded in the file (which is |
This is always true for parquet, because parquet has a different representation which we convert from and i don't think we can >create a dirty list from it, ever.
Correct. I don't believe it's possible to have dirty lists in parquet.
|
This PR implements equality comparator for LIST columns. This only supports "self" comparison for now, meaning the two rows to be compared should belong to the same table. A comparator that works on rows of two different tables will be implemented in another PR. This works only on "sanitized" list columns. See #10291 for details. This will partially support #10186. Authors: - Devavret Makkar (https://github.com/devavret) Approvers: - Robert Maynard (https://github.com/robertmaynard) - Vyas Ramasubramani (https://github.com/vyasr) - Mike Wilson (https://github.com/hyperbolic2346) - Jake Hemstad (https://github.com/jrhemstad) - Jordan Jacobelli (https://github.com/Ethyling) URL: #10289
This issue has been labeled |
Still needed. |
) Fixes #10291. With certain operations in `libcudf`, it is possible to produce `LIST` columns with `NULL` rows that are not also empty. For instance, consider a `STRUCT` column is constructed with an explicit validity buffer and a `LIST` child column: ```c++ auto const lists = lists_column_wrapper<int32_t>{ {0,1}, {2,3}, {4,5} }; auto const structs = structs_column_wrapper{ {lists}, null_at(1) }; ``` Since `structs[1] == NULL`, its `LIST` member is also deemed null. However, for efficiency, the null-ness is recorded in the `LIST`'s validity buffer, without purging the unnecessary values from its child. The `LIST` columns appears as follows: ``` Validity: 101 Offsets: [0, 2, 4, 6] Child: [0, 1, 2, 3, 4, 5] ``` Even though Row#1 is null, its size is `4-2 = 2`, and not `0`. (Row#1 is thus a non-empty null row.) This commit adds a `cudf::purge_nonempty_nulls()` function that purges such rows, and reduces such columns to a more space-efficient representation, i.e.: ``` Validity: 101 Offsets: [0, 2, 2, 4] Child: [0, 1, 4, 5] ``` This commit also modifies `cudf::gather()` not to produce `STRING`/`LIST` columns with "dirty" rows. Further, it adds two new functions to determine if a specified column needs such purging: 1. `cudf::may_have_nonempty_nulls()`: A fast check to check a column for the *possibility* of having non-empty nulls. This only checks whether the column or its descendants have null rows at all. If there are no nulls anywhere in the hierarchy, it does not need purging. 2. `cudf::has_nonempty_nulls()`: A deeper, more expensive check that categorically confirms whether non-empty null rows exist in any column in the hierarchy. Authors: - MithunR (https://github.com/mythrocks) Approvers: - Jake Hemstad (https://github.com/jrhemstad) - https://github.com/nvdbaranec - Jordan Jacobelli (https://github.com/Ethyling) URL: #10701
There are places in libcudf that expect a LIST column that has nulls in parent properly applied to the child.[1] i.e. if a list is null then it should also be empty and thus the offsets describing it should be same.
Similar to the
superimpose_parent_nulls
function for STRUCT columns, we need a function that can pushdown the nulls in a list column and compact a child when it had non-empty nulls.This is also a blocker for #10289
[1] Parquet writer depends on the nulls properly applied to leaf child. When writing leaf child columns of lists, the range to write in a particular page is determined using the offsets. e.g. if row 0 to 5 are to be written to page 1, then the selected children are offsets[0] to offset[5]. If row 2 is null but offset[2] < offset[2+1] then those extraneous children also end up in the page.
The text was updated successfully, but these errors were encountered: