[GitHub] [arrow] bkietz commented on a change in pull request #8894: ARROW-10322: [C++][Dataset] Minimize Expression

[GitBox]

bkietz commented on a change in pull request #8894:
URL: https://github.com/apache/arrow/pull/8894#discussion_r545383431



##########
File path: cpp/src/arrow/type.cc
##########
@@ -1039,13 +1042,31 @@ Result<std::shared_ptr<Field>> FieldPath::Get(const 
FieldVector& fields) const {
 
 Result<std::shared_ptr<Array>> FieldPath::Get(const RecordBatch& batch) const {
   ARROW_ASSIGN_OR_RAISE(auto data, FieldPathGetImpl::Get(this, 
batch.column_data()));
-  return MakeArray(data);
+  return MakeArray(std::move(data));
 }
 
 Result<std::shared_ptr<ChunkedArray>> FieldPath::Get(const Table& table) const 
{
   return FieldPathGetImpl::Get(this, table.columns());
 }
 
+Result<std::shared_ptr<Array>> FieldPath::Get(const Array& array) const {
+  ARROW_ASSIGN_OR_RAISE(auto data, Get(*array.data()));
+  return MakeArray(std::move(data));
+}
+
+Result<std::shared_ptr<ArrayData>> FieldPath::Get(const ArrayData& data) const 
{
+  return FieldPathGetImpl::Get(this, data.child_data);
+}
+
+Result<std::shared_ptr<ChunkedArray>> FieldPath::Get(const ChunkedArray& 
array) const {
+  FieldPath prefixed_with_0 = *this;
+  prefixed_with_0.indices_.insert(prefixed_with_0.indices_.begin(), 0);
+
+  ChunkedArrayVector vec;
+  vec.emplace_back(const_cast<ChunkedArray*>(&array), [](...) {});

Review comment:
       I'll probably just remove this. Retrieving fields from tables and 
chunked arrays is not currently used anywhere

##########
File path: cpp/src/arrow/type.h
##########
@@ -1423,10 +1426,14 @@ class ARROW_EXPORT FieldPath {
   Result<std::shared_ptr<Array>> Get(const RecordBatch& batch) const;
   Result<std::shared_ptr<ChunkedArray>> Get(const Table& table) const;
 
-  /// \brief Retrieve the referenced child Array from an Array or ChunkedArray
+  /// \brief Retrieve the referenced child from an Array, ArrayData, or 
ChunkedArray
   Result<std::shared_ptr<Array>> Get(const Array& array) const;
+  Result<std::shared_ptr<ArrayData>> Get(const ArrayData& data) const;
   Result<std::shared_ptr<ChunkedArray>> Get(const ChunkedArray& array) const;
 
+  /// \brief Retrieve the reference child from a Datum
+  Result<Datum> Get(const Datum& datum) const;

Review comment:
       I moved that to `expression_internal.h`, I'll remove this decl

##########
File path: cpp/src/arrow/type.h
##########
@@ -1423,10 +1426,14 @@ class ARROW_EXPORT FieldPath {
   Result<std::shared_ptr<Array>> Get(const RecordBatch& batch) const;
   Result<std::shared_ptr<ChunkedArray>> Get(const Table& table) const;
 
-  /// \brief Retrieve the referenced child Array from an Array or ChunkedArray
+  /// \brief Retrieve the referenced child from an Array, ArrayData, or 
ChunkedArray
   Result<std::shared_ptr<Array>> Get(const Array& array) const;
+  Result<std::shared_ptr<ArrayData>> Get(const ArrayData& data) const;
   Result<std::shared_ptr<ChunkedArray>> Get(const ChunkedArray& array) const;
 
+  /// \brief Retrieve the reference child from a Datum

Review comment:
       yes, will fix




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org