alamb commented on code in PR #9988:
URL: https://github.com/apache/arrow-datafusion/pull/9988#discussion_r1555586423
##########
datafusion/core/src/datasource/provider.rs:
##########
@@ -161,20 +161,34 @@ pub trait TableProvider: Sync + Send {
/// Specify if DataFusion should provide filter expressions to the
/// TableProvider to apply *during* the scan.
///
- /// The return value must have one element for each filter expression
passed
- /// in. The value of each element indicates if the TableProvider can apply
- /// that particular filter during the scan.
- ///
/// Some TableProviders can evaluate filters more efficiently than the
/// `Filter` operator in DataFusion, for example by using an index.
+ ///
+ /// The return value must have one element for each filter expression
passed
+ /// in. The value of each element indicates if the TableProvider can apply
+ /// that particular filter during the scan. The position in the return
value
+ /// Vec corresponds to the expression in the `filters` input. One approach
is to
+ /// allocate a result Vec equal to the size of the `filters` length, then
+ /// loop the `filters` array and indicate the proper level of support (or
not)
+ /// for each individual filter expression according to the capabilities
+ /// of the TableProvider.
Review Comment:
While accurate, I do wonder if the description of a loop in English
distracts from the rest of the text. I think it is worth considering omitting
that text like this
```suggestion
/// Vec corresponds to the expression in the `filters` input.
```
(and perhaps instead add an example showing the loop that is described)
--
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]
