[REVIEW] JSON host tree algorithms #16545
Conversation
github-actions
bot
added
libcudf
shrshi
added
cuIO
| } | ||
|
|
||
| // Pruning | ||
| thrust::host_vector<uint8_t> is_pruned(num_columns, options.is_enabled_prune_columns()); |
There was a problem hiding this comment.
Now it's still uint8_t, but assignments use true/false. We shold use the same type, whichever it is.
| dev_ref parent_ref = std::ref(root); | ||
| // creates children column | ||
| std::function<void(NodeIndexT, dev_ref)> construct_tree; | ||
| construct_tree = [&](NodeIndexT root, dev_ref ref) -> void { |
There was a problem hiding this comment.
I wonder if some of the lambdas in this function should be functions. They're pretty large and overall build_tree is definitely on the "too big" side. Not a blocking comment.
There was a problem hiding this comment.
I considered making it independent functions too. But the number of captures by reference seemed very high.
is there any smarter way to do this (like that dispatch function suggestion)?
There was a problem hiding this comment.
Maybe a (functor?) class, so common parameters are stored as data members and we don't have to pass them in each call. We would need to pass them explicitly to create the object, but at least it's done only once.
|
/merge |
Description
Depends on #16836
This change adds a new host tree building algorithms for JSON reader and utf8 field name support.
This constructs the device_column_tree using an adjacency list created from parent information.
This adjacency list is pruned based on input schema, and also types are enforced as per schema.
mark_is_prunedTree is constructed from pruned adjacency list, (with mixed types handling).
construct_treeutf8 field name support added: (spark requested)
utf8 decoding of field names during hashing of field nodes so that utf8 encoded field names also match to same column.
All unit tests passes, 1 unit test added where old algorithm fails.
This code is kept under experimental flag.
Checklist