milesgranger commented on code in PR #14697:
URL: https://github.com/apache/arrow/pull/14697#discussion_r1029371327
##########
cpp/src/arrow/type.cc:
##########
@@ -1066,12 +1066,32 @@ struct FieldPathGetImpl {
}
int depth = 0;
- const T* out;
+ const T* out = nullptr;
for (int index : path->indices()) {
if (children == nullptr) {
return Status::NotImplemented("Get child data of non-struct array");
}
+ if constexpr (std::is_same_v<T, std::shared_ptr<arrow::Field>>) {
+ // For lists, we don't care about the index, jump right into the list
value type.
+ // The index here is used in the kernel to grab the specific list
element.
+ // Children then are fields from list type, and out will be the
children from
+ // that field, thus index 0.
+ if (out != nullptr && (*out)->type()->id() == Type::LIST) {
+ children = get_children(*out);
+ index = 0;
+ } else if (out == nullptr && children->size() == 1 && index >= 0) {
+ // WIP: Perhaps a dangerous assumption?
+ // For list types when previous was not a FieldPath, but
+ // a string name referencing a list type. Then the first iteration
+ // with index (referencing the list item), might look like it's
+ // out of bounds when actually we don't care about this index for
+ // type checking, only the kernel does; we need the list value type
+ // which is now the only child in the children vector.
Review Comment:
Sure, this path happens with the `".a[1].c"` case, where the 'a' goes
through string name, and the second '[1]' is through this FieldPath, but is
also out of bounds from children (index 1) whereas `".a[0].c"` works b/c 0 is
the index we want anyway from the list type in order to get the list value
type.
I'll work on adding the FieldPath::Get tests in C++. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
