Remove unnecessary traits and wrapper types from the query crate

[evenyag]

What type of enhancement is this?

Refactor

What does the enhancement do?

We defined some wrapper types and traits for the query engine

• Expr refactor: replace Expr with datafusion::Expr #3995
• LogicalPlan refactor: remove DfPlan wrapper  #4733
• LogicalOptimizer refactor: Remove PhysicalOptimizer and LogicalOptimizer trait #4426
• PhysicalOptimizer refactor: Remove PhysicalOptimizer and LogicalOptimizer trait #4426
• PhysicalPlanner refactor(query): Remove PhysicalPlanner trait #4412
• PhysicalPlan refactor: Remove PhysicalPlan trait and use ExecutionPlan directly #3894

Most implementations simply forward requests to datafusion. Since we are highly coupled with datafusion and have no plan to support another query engine, we could remove these types.

Implementation challenges

For Expr:

greptimedb/src/common/query/src/logical_plan/expr.rs

Lines 22 to 27 in d997463

	/// Central struct of query API.
	/// Represent logical expressions such as `A + 1`, or `CAST(c1 AS int)`.
	#[derive(Clone, PartialEq, Eq, Hash, Debug)]
	pub struct Expr {
	df_expr: DfExpr,
	}

We could simply remove the Expr and re-export datafusion's Expr

pub use datafusion_expr::expr::Expr;

The LogicalPlan only has one variant so we can replace it with datafusion's LogicalPlan. But we also need to refactor methods of the enum so they can take datafusion's LogicalPlan as input.

greptimedb/src/query/src/plan.rs

Lines 35 to 38 in d997463

	#[derive(Clone, Debug)]
	pub enum LogicalPlan {
	DfPlan(DfLogicalPlan),
	}

We could also remove adapters like the DfPhysicalPlanAdapter if we remove the PhysicalPlan.

Many places use Expr, LogicalPlan, and PhysicalPlan. We might need to refactor them in different PRs.

[sunheyi6]

@evenyag i want to pr LogicalPlan part.I took a rough look. Is it to delete the query::plan file and replace all query::plan::LogicalPlan with datafusion_expr::LogicalPlan?

[evenyag]

@evenyag i want to pr LogicalPlan part.I took a rough look. Is it to delete the query::plan file and replace all query::plan::LogicalPlan with datafusion_expr::LogicalPlan?

We can still keep the query::plan file as we already implemented some methods for LogicalPlan. We may need to implement them as helper functions that take datafusion's plan as a parameter.

[sunheyi6]

@evenyag i want to pr LogicalPlan part.I took a rough look. Is it to delete the query::plan file and replace all query::plan::LogicalPlan with datafusion_expr::LogicalPlan?

We can still keep the query::plan file as we already implemented some methods for LogicalPlan. We may need to implement them as helper functions that take datafusion's plan as a parameter.

Could you please help me check if this modification meets your means?

before:

    pub fn schema(&self) -> Result<Schema> {
        match self {
            Self::DfPlan(plan) => {
                let df_schema = plan.schema();
                df_schema
                    .clone()
                    .try_into()
                    .context(ConvertDatafusionSchemaSnafu)
            }
        }
    }

after:

    pub fn schema(&self, plan: &DfLogicalPlan) -> Result<Schema> {
        let df_schema = plan.schema();
        df_schema.clone()
            .try_into()
            .context(ConvertDatafusionSchemaSnafu)
    }

[evenyag]

You should remove &self

fn plan_schema(plan: &DfLogicalPlan) -> Result<Schema> {
    let df_schema = plan.schema();
    df_schema.clone()
        .try_into()
        .context(ConvertDatafusionSchemaSnafu)
}

I also renamed the function to plan_schema() to indicate it returns a schema for a plan.

[leaf-potato]

@evenyag Remove the remaining 3 traits：

• PhysicalPlanner: refactor(query): Remove PhysicalPlanner trait #4412
• LogicalOptimizer & PhysicalOptimizer: refactor: Remove PhysicalOptimizer and LogicalOptimizer trait #4426