wiedld commented on code in PR #13986:
URL: https://github.com/apache/datafusion/pull/13986#discussion_r1914279888
##########
datafusion/physical-plan/src/execution_plan.rs:
##########
@@ -110,6 +110,16 @@ pub trait ExecutionPlan: Debug + DisplayAs + Send + Sync {
/// trait, which is implemented for all `ExecutionPlan`s.
fn properties(&self) -> &PlanProperties;
+ /// Returns an error if this individual node does not conform to its
invariants.
+ /// These invariants are typically only checked in debug mode.
+ ///
+ /// A default set of invariants is provided in the default implementation.
+ /// Extension nodes can provide their own invariants.
+ fn check_node_invariants(&self) -> Result<()> {
+ // TODO
Review Comment:
Thank you both for the reviews. Apologies on the delayed response.
To summarize [this nice
explanation](https://github.com/apache/datafusion/pull/13986#discussion_r1901204911)
from @ozankabak , the `Executable`-ness of the output plan (post optimizer
run) is dependent upon what each optimizer run does **_and_** if the input plan
was valid. Although it is surprising, we currently permit optimizer rules to
output invalid plans.
As such, I added a `PhysicalOptimizerRule::executable_check` which defines
the expected behavior per optimizer rule ([see commit
here](https://github.com/apache/datafusion/pull/13986/commits/5760792a7a4e051305adbeb2e84fd4d5c265e5ea)).
This also helps us surface which rules may produce unexecutable plans, as well
as when we can define an output plan as "executable".
Next, the `InvariantChecker` was expanded to [conditionally check the
executableness](https://github.com/apache/datafusion/pull/13986/commits/94482d1ab9091bfe1395e8e02c14bb07c59f40e4)
based upon the declared expectations. If the plan is expected to be
executable, then the invariants are checked for both (a) Datafusion internal
definition of "executable" from the sanity plan check, as well as (b) any
defined invariants on a user's ExecutionPlan extensions.
Finally, there is [an example test case
added](https://github.com/apache/datafusion/blob/ad15c85e229e2e1da9e54d3f2732e237c4760b0f/datafusion/core/src/physical_planner.rs#L3115-L3171)
which demonstrates how this could be useful for catching incompatibilities
with users' PhysicalOptimizerRule extensions.
--
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]
