You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "crepererum (via GitHub)" <gi...@apache.org> on 2023/02/27 15:11:32 UTC

[GitHub] [arrow-datafusion] crepererum opened a new pull request, #5419: refactor: ParquetExec logical expr. => phys. expr.

crepererum opened a new pull request, #5419:
URL: https://github.com/apache/arrow-datafusion/pull/5419

   # Which issue does this PR close?
   Closes #4695.
   
   # Rationale for this change
   Use `Arc<dyn PhysicalExpr>` instead of `Expr` within `ParquetExec` and move lowering from logical to physical expression into plan lowering (e.g. `ListingTable`).
   
   This is in line w/ all other physical plan nodes (e.g. `FilterExpr`) and simplifies reasoning within physical optimizer but also allows correct passing of `ExecutionProps` into the conversion.
   
   # What changes are included in this PR?
   Basically a bunch of type changes. The tests still use logical expressions because they are easier to construct and its a simple conversion from logical to phys. (at least in the test cases).
   
   # Are these changes tested?
   All existing tests pass. Removed tests that no longer apply. Added one to demonstrate `PruningPredicate` behavior when no columns are referenced (see https://github.com/apache/arrow-datafusion/pull/5386#discussion_r1117641476 ).
   
   # Are there any user-facing changes?
   - Phys. plan printouts look slightly differently.
   - **BREAKING:** Serialization has changed.
   - **BREAKING:** `ParquetExec` arguments changed.


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


[GitHub] [arrow-datafusion] alamb merged pull request #5419: refactor: ParquetExec logical expr. => phys. expr.

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #5419:
URL: https://github.com/apache/arrow-datafusion/pull/5419


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


[GitHub] [arrow-datafusion] crepererum commented on pull request #5419: refactor: ParquetExec logical expr. => phys. expr.

Posted by "crepererum (via GitHub)" <gi...@apache.org>.
crepererum commented on PR #5419:
URL: https://github.com/apache/arrow-datafusion/pull/5419#issuecomment-1446519622

   I've also fixed a few test expressions that where buggy (or at least would require type coercion which would run on the logical plan), most notably:
   
   - `negate(cast(some_int to UTF8))` => `cast(negate(some_int) to UTF8)`
   - `some_bool.and/or(some_int % 2)` => `some_bool.and/or(some_int % 2 == 0)`


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


[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #5419: refactor: ParquetExec logical expr. => phys. expr.

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #5419:
URL: https://github.com/apache/arrow-datafusion/pull/5419#discussion_r1120449504


##########
datafusion/core/src/physical_plan/file_format/parquet/row_groups.rs:
##########
@@ -324,12 +329,15 @@ mod tests {
     fn row_group_pruning_predicate_partial_expr() {
         use datafusion_expr::{col, lit};
         // test row group predicate with partially supported expression
-        // int > 1 and int % 2 => c1_max > 1 and true
-        let expr = col("c1").gt(lit(15)).and(col("c2").modulus(lit(2)));
+        // (int > 1) and ((int % 2) = 0) => c1_max > 1 and true

Review Comment:
   thank you - this is a good change



##########
datafusion/physical-expr/src/utils.rs:
##########
@@ -235,6 +239,80 @@ pub fn ordering_satisfy_concrete<F: FnOnce() -> EquivalenceProperties>(
     }
 }
 
+/// Extract referenced [`Column`]s within a [`PhysicalExpr`].
+///
+/// This works recursively.
+pub fn get_phys_expr_columns(pred: &Arc<dyn PhysicalExpr>) -> HashSet<Column> {
+    let mut rewriter = ColumnCollector::default();

Review Comment:
   👍  this is quite clever



##########
datafusion/core/src/datasource/file_format/mod.rs:
##########
@@ -84,7 +84,7 @@ pub trait FileFormat: Send + Sync + fmt::Debug {
         &self,
         state: &SessionState,
         conf: FileScanConfig,
-        filters: &[Expr],
+        filters: Option<&Arc<dyn PhysicalExpr>>,

Review Comment:
   I think it is a good change to have this take a single `PhysicalExpr` rather than a slice as it is more consistent with `TableProvider::scan` 👍 



##########
datafusion/core/src/physical_optimizer/pruning.rs:
##########
@@ -187,7 +170,7 @@ impl PruningPredicate {
         // in the predicate means we don't know for sure if the row
         // group can be filtered out or not. To maintain correctness
         // the row group must be kept and thus `true` is returned.
-        match self.predicate_expr.evaluate(&statistics_batch)? {
+        match self.predicate_expr.evaluate(&statistics_batch).expect("bug") {

Review Comment:
   I think it would be nicer here to return the error rather than panic, but I also think panic'ing is ok



##########
datafusion/core/src/physical_optimizer/pruning.rs:
##########
@@ -794,51 +853,81 @@ fn build_predicate_expression(
     build_statistics_expr(&mut expr_builder).unwrap_or(unhandled)
 }
 
-fn build_statistics_expr(expr_builder: &mut PruningExpressionBuilder) -> Result<Expr> {
-    let statistics_expr =
+fn build_statistics_expr(
+    expr_builder: &mut PruningExpressionBuilder,
+) -> Result<Arc<dyn PhysicalExpr>> {
+    let statistics_expr: Arc<dyn PhysicalExpr> =
         match expr_builder.op() {
             Operator::NotEq => {
                 // column != literal => (min, max) = literal =>
                 // !(min != literal && max != literal) ==>
                 // min != literal || literal != max
                 let min_column_expr = expr_builder.min_column_expr()?;
                 let max_column_expr = expr_builder.max_column_expr()?;
-                min_column_expr
-                    .not_eq(expr_builder.scalar_expr().clone())
-                    .or(expr_builder.scalar_expr().clone().not_eq(max_column_expr))
+                Arc::new(phys_expr::BinaryExpr::new(
+                    Arc::new(phys_expr::BinaryExpr::new(
+                        min_column_expr,
+                        Operator::NotEq,
+                        expr_builder.scalar_expr().clone(),
+                    )),
+                    Operator::Or,
+                    Arc::new(phys_expr::BinaryExpr::new(
+                        expr_builder.scalar_expr().clone(),
+                        Operator::NotEq,
+                        max_column_expr,
+                    )),
+                ))
             }
             Operator::Eq => {
                 // column = literal => (min, max) = literal => min <= literal && literal <= max
                 // (column / 2) = 4 => (column_min / 2) <= 4 && 4 <= (column_max / 2)
                 let min_column_expr = expr_builder.min_column_expr()?;
                 let max_column_expr = expr_builder.max_column_expr()?;
-                min_column_expr
-                    .lt_eq(expr_builder.scalar_expr().clone())
-                    .and(expr_builder.scalar_expr().clone().lt_eq(max_column_expr))
+                Arc::new(phys_expr::BinaryExpr::new(

Review Comment:
   it is unfortunate there isn't as nice "fluent" style APIs for building physical expressions. Maybe we can add a `PhysicalExprExt` trait that knew how to do so. 
   
   I'll file a ticket to track doing so as a follow on PR



##########
datafusion/core/src/physical_plan/file_format/parquet/row_filter.rs:
##########
@@ -211,16 +211,16 @@ impl<'a> FilterCandidateBuilder<'a> {
     }
 }
 
-impl<'a> ExprRewriter for FilterCandidateBuilder<'a> {
-    fn pre_visit(&mut self, expr: &Expr) -> Result<RewriteRecursion> {
-        if let Expr::Column(column) = expr {
-            if let Ok(idx) = self.file_schema.index_of(&column.name) {
+impl<'a> TreeNodeRewriter<Arc<dyn PhysicalExpr>> for FilterCandidateBuilder<'a> {

Review Comment:
   This is cool to see that there is an equivalent logical and physical rewriter



##########
datafusion/core/src/physical_optimizer/pruning.rs:
##########
@@ -128,31 +126,16 @@ impl PruningPredicate {
     /// For example, the filter expression `(column / 2) = 4` becomes
     /// the pruning predicate
     /// `(column_min / 2) <= 4 && 4 <= (column_max / 2))`
-    pub fn try_new(expr: Expr, schema: SchemaRef) -> Result<Self> {
+    pub fn try_new(expr: Arc<dyn PhysicalExpr>, schema: SchemaRef) -> Result<Self> {
         // build predicate expression once
         let mut required_columns = RequiredStatColumns::new();
-        let logical_predicate_expr =
+        let predicate_expr =
             build_predicate_expression(&expr, schema.as_ref(), &mut required_columns);
-        let stat_fields = required_columns
-            .iter()
-            .map(|(_, _, f)| f.clone())
-            .collect::<Vec<_>>();
-        let stat_schema = Schema::new(stat_fields);
-        let stat_dfschema = DFSchema::try_from(stat_schema.clone())?;
-
-        // TODO allow these properties to be passed in

Review Comment:
   👍  now it uses the real thing . Nice!



##########
datafusion/core/src/physical_optimizer/pruning.rs:
##########
@@ -1840,12 +2034,34 @@ mod tests {
         (schema, statistics, expected_true, expected_false)
     }
 
+    #[test]
+    fn prune_bool_const_expr() {
+        let (schema, statistics, _, _) = bool_setup();
+
+        // true
+        let expr = lit(true);
+        let expr = logical2physical(&expr, &schema);
+        let p = PruningPredicate::try_new(expr, schema.clone()).unwrap();
+        let result = p.prune(&statistics).unwrap();
+        assert_eq!(result, vec![true, true, true, true, true]);
+
+        // false
+        // constant literals that do NOT refer to any columns are currently not evaluated at all, hence the result is
+        // "all true"
+        let expr = lit(false);

Review Comment:
   this is unfortunate (an all false predicate should skip the entire file). However, as long as this PR doesn't make the pruning worse in this situation I think it is fine (and we can improve things in a follow on 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: github-unsubscribe@arrow.apache.org

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


[GitHub] [arrow-datafusion] crepererum commented on a diff in pull request #5419: refactor: ParquetExec logical expr. => phys. expr.

Posted by "crepererum (via GitHub)" <gi...@apache.org>.
crepererum commented on code in PR #5419:
URL: https://github.com/apache/arrow-datafusion/pull/5419#discussion_r1118884746


##########
datafusion/core/src/datasource/listing/table.rs:
##########
@@ -661,6 +663,20 @@ impl TableProvider for ListingTable {
             })
             .collect::<Result<Vec<_>>>()?;
 
+        let filters = if let Some(expr) = conjunction(filters.to_vec()) {
+            // NOTE: Use the table schema (NOT file schema) here because `expr` may contain references to partition columns.
+            let table_df_schema = self.table_schema.as_ref().clone().to_dfschema()?;
+            let filters = create_physical_expr(
+                &expr,
+                &table_df_schema,
+                &self.table_schema,
+                state.execution_props(),

Review Comment:
   Uses the real `ExeuctionProps` now :)



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


[GitHub] [arrow-datafusion] crepererum commented on a diff in pull request #5419: refactor: ParquetExec logical expr. => phys. expr.

Posted by "crepererum (via GitHub)" <gi...@apache.org>.
crepererum commented on code in PR #5419:
URL: https://github.com/apache/arrow-datafusion/pull/5419#discussion_r1120486601


##########
datafusion/core/src/physical_optimizer/pruning.rs:
##########
@@ -187,7 +170,7 @@ impl PruningPredicate {
         // in the predicate means we don't know for sure if the row
         // group can be filtered out or not. To maintain correctness
         // the row group must be kept and thus `true` is returned.
-        match self.predicate_expr.evaluate(&statistics_batch)? {
+        match self.predicate_expr.evaluate(&statistics_batch).expect("bug") {

Review Comment:
   ah, this is debug code, I HAVE TO remove this before merging.



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


[GitHub] [arrow-datafusion] crepererum commented on a diff in pull request #5419: refactor: ParquetExec logical expr. => phys. expr.

Posted by "crepererum (via GitHub)" <gi...@apache.org>.
crepererum commented on code in PR #5419:
URL: https://github.com/apache/arrow-datafusion/pull/5419#discussion_r1118885925


##########
datafusion/core/src/physical_plan/file_format/parquet/row_filter.rs:
##########
@@ -391,36 +391,14 @@ pub fn build_row_filter(
 mod test {
     use super::*;
     use arrow::datatypes::Field;
-    use datafusion_expr::{cast, col, lit};
+    use datafusion_common::ToDFSchema;
+    use datafusion_expr::{cast, col, lit, Expr};
+    use datafusion_physical_expr::create_physical_expr;
+    use datafusion_physical_expr::execution_props::ExecutionProps;
     use parquet::arrow::parquet_to_arrow_schema;
     use parquet::file::reader::{FileReader, SerializedFileReader};
     use rand::prelude::*;
 
-    // Assume a column expression for a column not in the table schema is a projected column and ignore it

Review Comment:
   This test is no longer relevant because you cannot lower such expression from logical to physical.



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


[GitHub] [arrow-datafusion] jiangzhx commented on pull request #5419: refactor: ParquetExec logical expr. => phys. expr.

Posted by "jiangzhx (via GitHub)" <gi...@apache.org>.
jiangzhx commented on PR #5419:
URL: https://github.com/apache/arrow-datafusion/pull/5419#issuecomment-1489851152

   This feature is not implemented: Physical plan does not support logical expression EXISTS ()
   https://github.com/apache/arrow-datafusion/issues/5789
   
   version: 19.0.0   worked.
   


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


[GitHub] [arrow-datafusion] ozankabak commented on a diff in pull request #5419: refactor: ParquetExec logical expr. => phys. expr.

Posted by "ozankabak (via GitHub)" <gi...@apache.org>.
ozankabak commented on code in PR #5419:
URL: https://github.com/apache/arrow-datafusion/pull/5419#discussion_r1121812665


##########
datafusion/physical-expr/src/utils.rs:
##########
@@ -235,6 +239,80 @@ pub fn ordering_satisfy_concrete<F: FnOnce() -> EquivalenceProperties>(
     }
 }
 
+/// Extract referenced [`Column`]s within a [`PhysicalExpr`].
+///
+/// This works recursively.
+pub fn get_phys_expr_columns(pred: &Arc<dyn PhysicalExpr>) -> HashSet<Column> {
+    let mut rewriter = ColumnCollector::default();

Review Comment:
   Yes, happy to help!



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


[GitHub] [arrow-datafusion] crepererum commented on a diff in pull request #5419: refactor: ParquetExec logical expr. => phys. expr.

Posted by "crepererum (via GitHub)" <gi...@apache.org>.
crepererum commented on code in PR #5419:
URL: https://github.com/apache/arrow-datafusion/pull/5419#discussion_r1121397443


##########
datafusion/physical-expr/src/utils.rs:
##########
@@ -235,6 +239,80 @@ pub fn ordering_satisfy_concrete<F: FnOnce() -> EquivalenceProperties>(
     }
 }
 
+/// Extract referenced [`Column`]s within a [`PhysicalExpr`].
+///
+/// This works recursively.
+pub fn get_phys_expr_columns(pred: &Arc<dyn PhysicalExpr>) -> HashSet<Column> {
+    let mut rewriter = ColumnCollector::default();

Review Comment:
   @ozankabak the issue w/ `Vec` is that you have a `O(n^2)` complexity in the number of used columns. In InfluxDB IOx we sometimes have schemas w/ over 200 columns and I'm somewhat worried that such a simple oversight quickly turns into a performance bug.



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


[GitHub] [arrow-datafusion] crepererum commented on a diff in pull request #5419: refactor: ParquetExec logical expr. => phys. expr.

Posted by "crepererum (via GitHub)" <gi...@apache.org>.
crepererum commented on code in PR #5419:
URL: https://github.com/apache/arrow-datafusion/pull/5419#discussion_r1121398269


##########
datafusion/core/src/physical_optimizer/pruning.rs:
##########
@@ -187,7 +170,7 @@ impl PruningPredicate {
         // in the predicate means we don't know for sure if the row
         // group can be filtered out or not. To maintain correctness
         // the row group must be kept and thus `true` is returned.
-        match self.predicate_expr.evaluate(&statistics_batch)? {
+        match self.predicate_expr.evaluate(&statistics_batch).expect("bug") {

Review Comment:
   done



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


[GitHub] [arrow-datafusion] crepererum commented on a diff in pull request #5419: refactor: ParquetExec logical expr. => phys. expr.

Posted by "crepererum (via GitHub)" <gi...@apache.org>.
crepererum commented on code in PR #5419:
URL: https://github.com/apache/arrow-datafusion/pull/5419#discussion_r1120489517


##########
datafusion/core/src/physical_optimizer/pruning.rs:
##########
@@ -1840,12 +2034,34 @@ mod tests {
         (schema, statistics, expected_true, expected_false)
     }
 
+    #[test]
+    fn prune_bool_const_expr() {
+        let (schema, statistics, _, _) = bool_setup();
+
+        // true
+        let expr = lit(true);
+        let expr = logical2physical(&expr, &schema);
+        let p = PruningPredicate::try_new(expr, schema.clone()).unwrap();
+        let result = p.prune(&statistics).unwrap();
+        assert_eq!(result, vec![true, true, true, true, true]);
+
+        // false
+        // constant literals that do NOT refer to any columns are currently not evaluated at all, hence the result is
+        // "all true"
+        let expr = lit(false);

Review Comment:
   It's not making things worse. This was the state of the art even before #5386, but I was asked here https://github.com/apache/arrow-datafusion/pull/5386#discussion_r1117641476 for the concrete semantics and I couldn't find a test that already covered that, so I've added one.



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


[GitHub] [arrow-datafusion] ozankabak commented on a diff in pull request #5419: refactor: ParquetExec logical expr. => phys. expr.

Posted by "ozankabak (via GitHub)" <gi...@apache.org>.
ozankabak commented on code in PR #5419:
URL: https://github.com/apache/arrow-datafusion/pull/5419#discussion_r1120950514


##########
datafusion/physical-expr/src/utils.rs:
##########
@@ -235,6 +239,80 @@ pub fn ordering_satisfy_concrete<F: FnOnce() -> EquivalenceProperties>(
     }
 }
 
+/// Extract referenced [`Column`]s within a [`PhysicalExpr`].
+///
+/// This works recursively.
+pub fn get_phys_expr_columns(pred: &Arc<dyn PhysicalExpr>) -> HashSet<Column> {
+    let mut rewriter = ColumnCollector::default();

Review Comment:
   Interesting! We had the same need (collecting columns) emerge in SHJ implementation, so we used this more lightweight recursion:
   ```rust
   fn collect_columns_recursive(expr: &Arc<dyn PhysicalExpr>, columns: &mut Vec<Column>) {
       if let Some(column) = expr.as_any().downcast_ref::<Column>() {
           if !columns.iter().any(|c| c.eq(column)) {
               columns.push(column.clone())
           }
       }
       expr.children()
           .iter()
           .for_each(|e| collect_columns_recursive(e, columns))
   }
   
   fn collect_columns(expr: &Arc<dyn PhysicalExpr>) -> Vec<Column> {
       let mut columns = vec![];
       collect_columns_recursive(expr, &mut columns);
       columns
   }
   ```
   We used a `Vec` instead of a `HashSet` due to anticipated small sizes, but the code is essentially the same 🙂
   
   This makes me think that doing a comprehensive code review and collecting/coalescing/documenting utilities such as this may simplify the codebase, and could be a worthy pursuit.



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


[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #5419: refactor: ParquetExec logical expr. => phys. expr.

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #5419:
URL: https://github.com/apache/arrow-datafusion/pull/5419#discussion_r1121562818


##########
datafusion/physical-expr/src/utils.rs:
##########
@@ -235,6 +239,80 @@ pub fn ordering_satisfy_concrete<F: FnOnce() -> EquivalenceProperties>(
     }
 }
 
+/// Extract referenced [`Column`]s within a [`PhysicalExpr`].
+///
+/// This works recursively.
+pub fn get_phys_expr_columns(pred: &Arc<dyn PhysicalExpr>) -> HashSet<Column> {
+    let mut rewriter = ColumnCollector::default();

Review Comment:
   > This makes me think that doing a comprehensive code review and collecting/coalescing/documenting utilities such as this may simplify the codebase, and could be a worthy pursuit.
   
   This absolutely would be a worthy pursuit -- is that something you could help lead @ozankabak ?



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


[GitHub] [arrow-datafusion] ursabot commented on pull request #5419: refactor: ParquetExec logical expr. => phys. expr.

Posted by "ursabot (via GitHub)" <gi...@apache.org>.
ursabot commented on PR #5419:
URL: https://github.com/apache/arrow-datafusion/pull/5419#issuecomment-1450449643

   Benchmark runs are scheduled for baseline = c4b895822a02d28d9adcd42112cc2604dde5d0e1 and contender = 03fbf9fecad00f8d6eb3e72e72ba16252b28b1d6. 03fbf9fecad00f8d6eb3e72e72ba16252b28b1d6 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ec2-t3-xlarge-us-east-2] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/bfa8fdd2d8424bc09ccaf799adea02e6...ddab7ca7b40a4ea28d10bdf4c4b4221b/)
   [Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on test-mac-arm] [test-mac-arm](https://conbench.ursa.dev/compare/runs/31dbe55bb88a430e9fef8d99eebb1424...7a7fd81653a04f5a8c6b328022315aa1/)
   [Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ursa-i9-9960x] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/8c0736b67b344f5c9b61d50067c3d5c2...45c2f3426a3b4a7e8ceb19ed88f6c207/)
   [Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ursa-thinkcentre-m75q] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/d8d611bb731845f0b68b821c442d81b9...94ce6c80d79b48f6b1bf11615ab5ee7b/)
   Buildkite builds:
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
   test-mac-arm: Supported benchmark langs: C++, Python, R
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


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


[GitHub] [arrow-datafusion] alamb commented on pull request #5419: refactor: ParquetExec logical expr. => phys. expr.

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #5419:
URL: https://github.com/apache/arrow-datafusion/pull/5419#issuecomment-1481807475

   This PR appears to have introduced a regression -- see https://github.com/apache/arrow-datafusion/issues/5708


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