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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
