alamb commented on code in PR #9988:
URL: https://github.com/apache/arrow-datafusion/pull/9988#discussion_r1556334496


##########
datafusion/core/src/datasource/provider.rs:
##########
@@ -161,20 +161,71 @@ 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.
-    ///
-    /// By default, returns [`Unsupported`] for all filters, meaning no filters
+    /// 
+    /// 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. 
+    /// 
+    /// Here is an example of how this can be done:
+    /// 
+    /// ```rust
+    /// use datafusion::error::{Result, DataFusionError};
+    /// use datafusion_expr::{Expr, TableProviderFilterPushDown};
+    /// 
+    /// fn supports_filters_pushdown(
+    ///     &self,
+    ///     filters: &[&Expr],) -> Result<Vec<TableProviderFilterPushDown>> {
+    /// 
+    ///     let mut result_vec: Vec<TableProviderFilterPushDown> = 
Vec::with_capacity(&filters.len());
+    /// 
+    ///     for i in 0..filters.len() {
+    ///         let expr = *filters[i];
+    /// 
+    ///         match expr {
+    ///             Expr::Between(between_expr) => {
+    ///                 match between_expr.expr.try_into_col().ok() {
+    ///                     Some(column) => {
+    ///                         let column_name = column.name;
+    ///                         if column_name = "c1".to_string() {
+    ///                             
result_vec.push(TableProviderFilterPushDown::Exact);
+    ///                         } else {
+    ///                             
result_vec.push(TableProviderFilterPushDown::Unsupported);
+    ///                         }
+    ///                     } 
+    ///                     None => {
+    ///                         return 
Err(DataFusionError::Execution(format!("Could not get column. between_expr: 
{:?}", between_expr)));
+    ///                     }

Review Comment:
   The example might be more general if it didn't error but instead returned 
`Unsupported` for other exprs
   
   Also, if you want to get your functional Rust programming on, you can use 
something like this (untested):
   
   ```rust
     between_expr.expr.try_into_col()
        .map(|column| {
           if &column.name = "c1" {
             TableProviderFilterPushDown::Exact
           } else {
             TableProviderFilterPushDown::Unsupported
           }
        })
        // if not a column reference, return unsupported
       .unwrap_or(TableProviderFilterPushDown::Unsupported)
   ```



##########
datafusion/core/src/datasource/provider.rs:
##########
@@ -161,20 +161,71 @@ 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.
-    ///
-    /// By default, returns [`Unsupported`] for all filters, meaning no filters
+    /// 
+    /// 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. 
+    /// 
+    /// Here is an example of how this can be done:
+    /// 
+    /// ```rust
+    /// use datafusion::error::{Result, DataFusionError};
+    /// use datafusion_expr::{Expr, TableProviderFilterPushDown};
+    /// 
+    /// fn supports_filters_pushdown(
+    ///     &self,
+    ///     filters: &[&Expr],) -> Result<Vec<TableProviderFilterPushDown>> {
+    /// 
+    ///     let mut result_vec: Vec<TableProviderFilterPushDown> = 
Vec::with_capacity(&filters.len());
+    /// 
+    ///     for i in 0..filters.len() {
+    ///         let expr = *filters[i];
+    /// 
+    ///         match expr {

Review Comment:
   For examples, I think it helps to add a comment explaining what it is doing 
for new readers
   for example, here you might add:
   
   ```suggestion
       ///         // Return `Exact` if we have a way to evaluate the 
expression exactly. In this
       ///         // we have a way to evaluate expressions on column c1, but 
not other columns, 
       ///         match expr {
   ```



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

Reply via email to