[GitHub] [arrow] westonpace commented on a diff in pull request #13804: ARROW-17318: [C++][Dataset] Support async streaming interface for getting fragments in Dataset

[GitBox <gi...@apache.org>]

westonpace commented on code in PR #13804:
URL: https://github.com/apache/arrow/pull/13804#discussion_r938975758


##########
cpp/src/arrow/dataset/dataset.cc:
##########
@@ -160,6 +160,26 @@ Result<FragmentIterator> Dataset::GetFragments(compute::Expression predicate) {
                                    : MakeEmptyIterator<std::shared_ptr<Fragment>>();
 }
 
+Result<FragmentGenerator> Dataset::GetFragmentsAsync() {
+  return GetFragmentsAsync(compute::literal(true));
+}
+
+Result<FragmentGenerator> Dataset::GetFragmentsAsync(compute::Expression predicate) {
+  ARROW_ASSIGN_OR_RAISE(
+      predicate, SimplifyWithGuarantee(std::move(predicate), partition_expression_));
+  return predicate.IsSatisfiable()
+             ? GetFragmentsAsyncImpl(std::move(predicate))
+             : MakeEmptyGenerator<FragmentGenerator::result_type::ValueType>();

Review Comment:
   ```suggestion
                : MakeEmptyGenerator<std::shared_ptr<Fragment>>();
   ```



##########
cpp/src/arrow/dataset/dataset.h:
##########
@@ -134,6 +135,9 @@ class ARROW_DS_EXPORT InMemoryFragment : public Fragment {
 
 /// @}
 
+using FragmentPtr = std::shared_ptr<Fragment>;

Review Comment:
   ```suggestion
   using FragmentPtr = std::shared_ptr<Fragment>;
   ```
   
   I think we generally try and avoid using too many `using` statements like this as it can make the code a bit harder to read for newcomers.  `FragmentGenerator` is probably ok as we have some precedent there.



-- 
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: github-unsubscribe@arrow.apache.org

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