findepi commented on code in PR #13651:
URL: https://github.com/apache/datafusion/pull/13651#discussion_r1874536239
##########
datafusion/optimizer/src/optimizer.rs:
##########
@@ -451,6 +468,33 @@ impl Optimizer {
}
}
+/// These are invariants to hold true for each logical plan.
+/// Do necessary check and fail the invalid plan.
+///
+/// Checks for elements which are immutable across optimizer passes.
+fn check_plan(
+ check_name: &str,
+ plan: &LogicalPlan,
+ prev_schema: Arc<DFSchema>,
+) -> Result<()> {
+ // verify invariant: optimizer rule didn't change the schema
+ assert_schema_is_the_same(check_name, &prev_schema, plan)?;
+
+ // verify invariant: fields must have unique names
+ assert_unique_field_names(plan)?;
+
+ /* This current fails for:
+ - execution::context::tests::cross_catalog_access
+ - at test_files/string/string.slt:46
+ External error: query failed: DataFusion error: Optimizer rule
'eliminate_nested_union' failed
+ */
+ // verify invariant: equivalent schema across union inputs
+ // assert_unions_are_valid(check_name, plan)?;
+
+ // TODO: trait API and provide extension on the Optimizer to define own
validations?
Review Comment:
> We have some special invariants for our `SortPreservingMerge` replacement,
`ProgressiveEval` (related to time ranges of parquet files) that would be great
to be able to encode
is this about `LogicalPlan::Extension`? I agree it makes sense to support
validation of these if we validate the overall plan.
--
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]
