bkietz commented on a change in pull request #11384:
URL: https://github.com/apache/arrow/pull/11384#discussion_r741213074
##########
File path: cpp/src/arrow/compute/kernels/vector_sort.cc
##########
@@ -51,6 +51,19 @@ struct SortField {
SortOrder order;
};
+Status CheckNonNested(const FieldRef& ref) {
+ if (ref.IsNested()) {
+ return Status::KeyError("Nested keys not supported for SortKeys");
Review comment:
`FieldRef` supports referencing fields of fields, for example
`FieldRef::FromDotPath('.alpha.beta')` could be applied to the schema
```
schema({
field("alpha", struct_({
field("beta", int32()),
})),
})
```
to acquire the inner field. Supporting that here would require refactoring
`vector_sort.cc` to not assume that field references can be expressed as a
single `int field_index` and instead support a path of field indices. I think
that's out of scope for this PR
##########
File path: cpp/src/arrow/compute/kernels/vector_sort.cc
##########
@@ -51,6 +51,19 @@ struct SortField {
SortOrder order;
};
+Status CheckNonNested(const FieldRef& ref) {
+ if (ref.IsNested()) {
+ return Status::KeyError("Nested keys not supported for SortKeys");
Review comment:
`FieldRef` supports referencing fields of fields, for example
`FieldRef::FromDotPath('.alpha.beta')` could be applied to the schema
```c++
schema({
field("alpha", struct_({
field("beta", int32()),
})),
})
```
to acquire the inner field. Supporting that here would require refactoring
`vector_sort.cc` to not assume that field references can be expressed as a
single `int field_index` and instead support a path of field indices. I think
that's out of scope for this PR
--
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]
