milesgranger commented on code in PR #14697:
URL: https://github.com/apache/arrow/pull/14697#discussion_r1029394880
##########
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:
The first 'a', in our example, will be matched [here in the second
overloaded member
function](https://github.com/apache/arrow/blob/7ae4705c629655eb37b4200229d30f3a2af9e154/cpp/src/arrow/type.cc#L1407)
then the second (`[1]`) goes to the first overloaded function taking
`FieldPath`.
Edit: possible I'm grossly misunderstanding something though. :)
--
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]
