Skip to content

Commit

Permalink
Require Debug for AnalyzerRule, FunctionRewriter, and `Optimize…
Browse files Browse the repository at this point in the history
…rRule` (#12556)
  • Loading branch information
alamb authored Sep 23, 2024
1 parent 99b5673 commit 2e274bf
Show file tree
Hide file tree
Showing 34 changed files with 45 additions and 32 deletions.
1 change: 1 addition & 0 deletions datafusion-examples/examples/optimizer_rule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ pub async fn main() -> Result<()> {

/// An example OptimizerRule that replaces all `col = <const>` predicates with a
/// user defined function
#[derive(Default, Debug)]
struct MyOptimizerRule {}

impl OptimizerRule for MyOptimizerRule {
Expand Down
1 change: 1 addition & 0 deletions datafusion/core/src/execution/session_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1954,6 +1954,7 @@ mod tests {

#[test]
fn test_session_state_with_optimizer_rules() {
#[derive(Default, Debug)]
struct DummyRule {}

impl OptimizerRule for DummyRule {
Expand Down
3 changes: 3 additions & 0 deletions datafusion/core/tests/user_defined/user_defined_plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,9 @@ impl QueryPlanner for TopKQueryPlanner {
}
}

#[derive(Default, Debug)]
struct TopKOptimizerRule {}

impl OptimizerRule for TopKOptimizerRule {
fn name(&self) -> &str {
"topk"
Expand Down Expand Up @@ -686,6 +688,7 @@ impl RecordBatchStream for TopKReader {
}
}

#[derive(Default, Debug)]
struct MyAnalyzerRule {}

impl AnalyzerRule for MyAnalyzerRule {
Expand Down
3 changes: 2 additions & 1 deletion datafusion/expr/src/expr_rewriter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

use std::collections::HashMap;
use std::collections::HashSet;
use std::fmt::Debug;
use std::sync::Arc;

use crate::expr::{Alias, Sort, Unnest};
Expand All @@ -42,7 +43,7 @@ pub use order_by::rewrite_sort_cols_by_aggs;
/// `Operator::ArrowAt`, but can be implemented by calling a function
/// `array_concat` from the `functions-nested` crate.
// This is not used in datafusion internally, but it is still helpful for downstream project so don't remove it.
pub trait FunctionRewrite {
pub trait FunctionRewrite: Debug {
/// Return a human readable name for this rewrite
fn name(&self) -> &str;

Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/analyzer/count_wildcard_rule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use datafusion_expr::{lit, Expr, LogicalPlan, WindowFunctionDefinition};
/// Rewrite `Count(Expr:Wildcard)` to `Count(Expr:Literal)`.
///
/// Resolves issue: <https://github.com/apache/datafusion/issues/5473>
#[derive(Default)]
#[derive(Default, Debug)]
pub struct CountWildcardRule {}

impl CountWildcardRule {
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/analyzer/expand_wildcard_rule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use datafusion_expr::utils::{
};
use datafusion_expr::{Expr, LogicalPlan, Projection, SubqueryAlias};

#[derive(Default)]
#[derive(Default, Debug)]
pub struct ExpandWildcardRule {}

impl ExpandWildcardRule {
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/analyzer/function_rewrite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use datafusion_expr::LogicalPlan;
use std::sync::Arc;

/// Analyzer rule that invokes [`FunctionRewrite`]s on expressions
#[derive(Default)]
#[derive(Default, Debug)]
pub struct ApplyFunctionRewrites {
/// Expr --> Function writes to apply
function_rewrites: Vec<Arc<dyn FunctionRewrite + Send + Sync>>,
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/analyzer/inline_table_scan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use datafusion_expr::{logical_plan::LogicalPlan, Expr, LogicalPlanBuilder};

/// Analyzed rule that inlines TableScan that provide a [`LogicalPlan`]
/// (DataFrame / ViewTable)
#[derive(Default)]
#[derive(Default, Debug)]
pub struct InlineTableScan;

impl InlineTableScan {
Expand Down
6 changes: 4 additions & 2 deletions datafusion/optimizer/src/analyzer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
// under the License.

//! [`Analyzer`] and [`AnalyzerRule`]

use std::fmt::Debug;
use std::sync::Arc;

use log::debug;
Expand Down Expand Up @@ -60,7 +62,7 @@ pub mod type_coercion;
/// `AnalyzerRule`s.
///
/// [`SessionState::add_analyzer_rule`]: https://docs.rs/datafusion/latest/datafusion/execution/session_state/struct.SessionState.html#method.add_analyzer_rule
pub trait AnalyzerRule {
pub trait AnalyzerRule: Debug {
/// Rewrite `plan`
fn analyze(&self, plan: LogicalPlan, config: &ConfigOptions) -> Result<LogicalPlan>;

Expand All @@ -72,7 +74,7 @@ pub trait AnalyzerRule {
///
/// An `Analyzer` transforms a `LogicalPlan`
/// prior to the rest of the DataFusion optimization process.
#[derive(Clone)]
#[derive(Clone, Debug)]
pub struct Analyzer {
/// Expr --> Function writes to apply prior to analysis passes
pub function_rewrites: Vec<Arc<dyn FunctionRewrite + Send + Sync>>,
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/analyzer/type_coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ use datafusion_expr::{

/// Performs type coercion by determining the schema
/// and performing the expression rewrites.
#[derive(Default)]
#[derive(Default, Debug)]
pub struct TypeCoercion {}

impl TypeCoercion {
Expand Down
1 change: 1 addition & 0 deletions datafusion/optimizer/src/common_subexpr_eliminate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ type CommonExprs<'n> = IndexMap<Identifier<'n>, (Expr, String)>;
/// ProjectionExec(exprs=[extract (day from new_col), extract (year from new_col)]) <-- reuse here
/// ProjectionExec(exprs=[to_date(c1) as new_col]) <-- compute to_date once
/// ```
#[derive(Debug)]
pub struct CommonSubexprEliminate {
random_state: RandomState,
}
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/decorrelate_predicate_subquery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ use datafusion_expr::{
use log::debug;

/// Optimizer rule for rewriting predicate(IN/EXISTS) subquery to left semi/anti joins
#[derive(Default)]
#[derive(Default, Debug)]
pub struct DecorrelatePredicateSubquery {}

impl DecorrelatePredicateSubquery {
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/eliminate_cross_join.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use datafusion_expr::logical_plan::{
use datafusion_expr::utils::{can_hash, find_valid_equijoin_key_pair};
use datafusion_expr::{build_join_schema, ExprSchemable, Operator};

#[derive(Default)]
#[derive(Default, Debug)]
pub struct EliminateCrossJoin;

impl EliminateCrossJoin {
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/eliminate_duplicated_expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use datafusion_expr::{Aggregate, Expr, Sort, SortExpr};
use indexmap::IndexSet;
use std::hash::{Hash, Hasher};
/// Optimization rule that eliminate duplicated expr.
#[derive(Default)]
#[derive(Default, Debug)]
pub struct EliminateDuplicatedExpr;

impl EliminateDuplicatedExpr {
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/eliminate_filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use crate::{OptimizerConfig, OptimizerRule};
///
/// This saves time in planning and executing the query.
/// Note that this rule should be applied after simplify expressions optimizer rule.
#[derive(Default)]
#[derive(Default, Debug)]
pub struct EliminateFilter;

impl EliminateFilter {
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/eliminate_group_by_constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use datafusion_expr::{Aggregate, Expr, LogicalPlan, LogicalPlanBuilder, Volatili
/// Optimizer rule that removes constant expressions from `GROUP BY` clause
/// and places additional projection on top of aggregation, to preserve
/// original schema
#[derive(Default)]
#[derive(Default, Debug)]
pub struct EliminateGroupByConstant {}

impl EliminateGroupByConstant {
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/eliminate_join.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use datafusion_expr::{

/// Eliminates joins when join condition is false.
/// Replaces joins when inner join condition is true with a cross join.
#[derive(Default)]
#[derive(Default, Debug)]
pub struct EliminateJoin;

impl EliminateJoin {
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/eliminate_limit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use std::sync::Arc;
/// plan with an empty relation.
///
/// This rule also removes OFFSET 0 from the [LogicalPlan]
#[derive(Default)]
#[derive(Default, Debug)]
pub struct EliminateLimit;

impl EliminateLimit {
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/eliminate_nested_union.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use datafusion_expr::{Distinct, LogicalPlan, Union};
use itertools::Itertools;
use std::sync::Arc;

#[derive(Default)]
#[derive(Default, Debug)]
/// An optimization rule that replaces nested unions with a single union.
pub struct EliminateNestedUnion;

Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/eliminate_one_union.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use std::sync::Arc;

use crate::optimizer::ApplyOrder;

#[derive(Default)]
#[derive(Default, Debug)]
/// An optimization rule that eliminates union with one element.
pub struct EliminateOneUnion;

Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/eliminate_outer_join.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ use datafusion_common::tree_node::Transformed;
use datafusion_expr::expr::{BinaryExpr, Cast, TryCast};
use std::sync::Arc;

#[derive(Default)]
///
/// Attempt to replace outer joins with inner joins.
///
Expand All @@ -49,6 +48,7 @@ use std::sync::Arc;
/// filters from the WHERE clause return false while any inputs are
/// null and columns of those quals are come from nullable side of
/// outer join.
#[derive(Default, Debug)]
pub struct EliminateOuterJoin;

impl EliminateOuterJoin {
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/extract_equijoin_predicate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ type EquijoinPredicate = (Expr, Expr);
/// has one equijoin predicate (`A.x = B.y`) and one filter predicate (`B.z > 50`).
/// See [find_valid_equijoin_key_pair] for more information on what predicates
/// are considered equijoins.
#[derive(Default)]
#[derive(Default, Debug)]
pub struct ExtractEquijoinPredicate;

impl ExtractEquijoinPredicate {
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/filter_null_join_keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use std::sync::Arc;
/// The FilterNullJoinKeys rule will identify joins with equi-join conditions
/// where the join key is nullable and then insert an `IsNotNull` filter on the nullable side since null values
/// can never match.
#[derive(Default)]
#[derive(Default, Debug)]
pub struct FilterNullJoinKeys {}

impl OptimizerRule for FilterNullJoinKeys {
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/optimize_projections/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ use datafusion_common::tree_node::{
/// The rule analyzes the input logical plan, determines the necessary column
/// indices, and then removes any unnecessary columns. It also removes any
/// unnecessary projections from the plan tree.
#[derive(Default)]
#[derive(Default, Debug)]
pub struct OptimizeProjections {}

impl OptimizeProjections {
Expand Down
8 changes: 6 additions & 2 deletions datafusion/optimizer/src/optimizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
//! [`Optimizer`] and [`OptimizerRule`]

use std::collections::HashSet;
use std::fmt::Debug;
use std::sync::Arc;

use chrono::{DateTime, Utc};
Expand Down Expand Up @@ -70,7 +71,7 @@ use crate::utils::log_plan;
/// [`AnalyzerRule`]: crate::analyzer::AnalyzerRule
/// [`SessionState::add_optimizer_rule`]: https://docs.rs/datafusion/latest/datafusion/execution/session_state/struct.SessionState.html#method.add_optimizer_rule

pub trait OptimizerRule {
pub trait OptimizerRule: Debug {
/// Try and rewrite `plan` to an optimized form, returning None if the plan
/// cannot be optimized by this rule.
///
Expand Down Expand Up @@ -214,7 +215,7 @@ impl OptimizerConfig for OptimizerContext {
}

/// A rule-based optimizer.
#[derive(Clone)]
#[derive(Clone, Debug)]
pub struct Optimizer {
/// All optimizer rules to apply
pub rules: Vec<Arc<dyn OptimizerRule + Send + Sync>>,
Expand Down Expand Up @@ -666,6 +667,7 @@ mod tests {

fn observe(_plan: &LogicalPlan, _rule: &dyn OptimizerRule) {}

#[derive(Default, Debug)]
struct BadRule {}

impl OptimizerRule for BadRule {
Expand All @@ -687,6 +689,7 @@ mod tests {
}

/// Replaces whatever plan with a single table scan
#[derive(Default, Debug)]
struct GetTableScanRule {}

impl OptimizerRule for GetTableScanRule {
Expand All @@ -713,6 +716,7 @@ mod tests {
/// A goofy rule doing rotation of columns in all projections.
///
/// Useful to test cycle detection.
#[derive(Default, Debug)]
struct RotateProjectionRule {
// reverse exprs instead of rotating on the first pass
reverse_on_first_pass: Mutex<bool>,
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/propagate_empty_relation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use crate::optimizer::ApplyOrder;
use crate::{OptimizerConfig, OptimizerRule};

/// Optimization rule that bottom-up to eliminate plan by propagating empty_relation.
#[derive(Default)]
#[derive(Default, Debug)]
pub struct PropagateEmptyRelation;

impl PropagateEmptyRelation {
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/push_down_filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ use crate::{OptimizerConfig, OptimizerRule};
/// reaches a plan node that does not commute with that filter, it adds the
/// filter to that place. When it passes through a projection, it re-writes the
/// filter's expression taking into account that projection.
#[derive(Default)]
#[derive(Default, Debug)]
pub struct PushDownFilter {}

/// For a given JOIN type, determine whether each input of the join is preserved
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/push_down_limit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use datafusion_expr::logical_plan::{Join, JoinType, Limit, LogicalPlan};
/// Optimization rule that tries to push down `LIMIT`.
///
//. It will push down through projection, limits (taking the smaller limit)
#[derive(Default)]
#[derive(Default, Debug)]
pub struct PushDownLimit {}

impl PushDownLimit {
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/replace_distinct_aggregate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ use datafusion_expr::{Aggregate, Distinct, DistinctOn, Expr, LogicalPlan};
/// ```

/// Optimizer that replaces logical [[Distinct]] with a logical [[Aggregate]]
#[derive(Default)]
#[derive(Default, Debug)]
pub struct ReplaceDistinctWithAggregate {}

impl ReplaceDistinctWithAggregate {
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/rewrite_disjunctive_predicate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ use datafusion_expr::{Expr, LogicalPlan, Operator};
/// )
/// ```
///
#[derive(Default)]
#[derive(Default, Debug)]
pub struct RewriteDisjunctivePredicate;

impl RewriteDisjunctivePredicate {
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/scalar_subquery_to_join.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ use datafusion_expr::utils::conjunction;
use datafusion_expr::{expr, EmptyRelation, Expr, LogicalPlan, LogicalPlanBuilder};

/// Optimizer rule for rewriting subquery filters to joins
#[derive(Default)]
#[derive(Default, Debug)]
pub struct ScalarSubqueryToJoin {}

impl ScalarSubqueryToJoin {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ use super::ExprSimplifier;
/// `Filter: b > 2`
///
/// [`Expr`]: datafusion_expr::Expr
#[derive(Default)]
#[derive(Default, Debug)]
pub struct SimplifyExpressions {}

impl OptimizerRule for SimplifyExpressions {
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/single_distinct_to_groupby.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ use hashbrown::HashSet;
/// )
/// GROUP BY a
/// ```
#[derive(Default)]
#[derive(Default, Debug)]
pub struct SingleDistinctToGroupBy {}

const SINGLE_DISTINCT_ALIAS: &str = "alias1";
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/unwrap_cast_in_comparison.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ use datafusion_expr::{lit, Expr, ExprSchemable, LogicalPlan};
/// Filter: c1 > INT32(10)
/// ```
///
#[derive(Default)]
#[derive(Default, Debug)]
pub struct UnwrapCastInComparison {}

impl UnwrapCastInComparison {
Expand Down

0 comments on commit 2e274bf

Please sign in to comment.