alamb commented on code in PR #17189: URL: https://github.com/apache/datafusion/pull/17189#discussion_r2277004846
########## datafusion/expr/src/logical_plan/extension.rs: ########## @@ -57,7 +57,7 @@ pub trait UserDefinedLogicalNode: fmt::Debug + Send + Sync { fn schema(&self) -> &DFSchemaRef; /// Perform check of invariants for the extension node. - fn check_invariants(&self, check: InvariantLevel, plan: &LogicalPlan) -> Result<()>; + fn check_invariants(&self, check: InvariantLevel) -> Result<()>; Review Comment: I think technically this is an API change. Given this is a public API I worry that this change might break downstream users and there is no other way to get at the plan ########## datafusion/pruning/src/pruning_predicate.rs: ########## @@ -978,6 +978,7 @@ impl<'a> PruningExpressionBuilder<'a> { } }; + // TODO pass in SchemaRef so we don't need to clone the schema Review Comment: I bet this would be a good first issue type ticket ########## datafusion/common/src/dfschema.rs: ########## @@ -899,6 +899,7 @@ impl TryFrom<SchemaRef> for DFSchema { field_qualifiers: vec![None; field_count], functional_dependencies: FunctionalDependencies::empty(), }; + dfschema.check_names()?; Review Comment: Context is that checking names here makes this consistent with the other (fallible) ways to create a DFSchema from a Schema ########## datafusion-examples/examples/pruning.rs: ########## @@ -187,10 +187,10 @@ impl PruningStatistics for MyCatalog { } fn create_pruning_predicate(expr: Expr, schema: &SchemaRef) -> PruningPredicate { - let df_schema = DFSchema::try_from(schema.as_ref().clone()).unwrap(); + let df_schema = DFSchema::try_from(Arc::clone(schema)).unwrap(); Review Comment: this saves a deep `clone` I think 👍 -- 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-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org