How to migrate Expression and ExpressionExecutor in ClusterStatsGenerator?

[Xuanwo]

Hi, I'm working on removing old planners. More specifically, I'm try to removing sqlparser.

However, there are some code in storage still depends on sqlparser's Expression:

#[derive(Clone, Default)]
pub struct ClusterStatsGenerator {
    cluster_key_id: u32,
    cluster_key_index: Vec<usize>,
    expression_executor: Option<ExpressionExecutor>,
    level: i32,
    block_compactor: BlockCompactor,
}

// This can be used in deletion, for an existing block.
pub fn gen_with_origin_stats(
    &self,
    data_block: &DataBlock,
    origin_stats: Option<ClusterStatistics>,
) -> Result<Option<ClusterStatistics>> {
    if origin_stats.is_none() {
        return Ok(None);
    }

    let origin_stats = origin_stats.unwrap();
    if origin_stats.cluster_key_id != self.cluster_key_id {
        return Ok(None);
    }

    let block = if let Some(executor) = &self.expression_executor {
        // For a clustered table, data_block has been sorted, but may not contain cluster key.
        // So only need to get the first and the last row for execute.
        let indices = vec![0u32, data_block.num_rows() as u32 - 1];
        let input = DataBlock::block_take_by_indices(data_block, &indices)?;
        executor.execute(&input)?
    } else {
        data_block.clone()
    };

    self.clusters_statistics(&block, origin_stats.level)
}

The cluster_key here is table's column name parsed by:

async fn analyze_cluster_keys(
    &mut self,
    cluster_by: &[Expr<'a>],
    schema: DataSchemaRef,
) -> Result<Vec<String>> { .. }

ClusterStatsGenerator will use expression_executor to remove unused columns.

---

How can I remove the Expression here?

[BohuTANG]

cc @zhyass for suggestions

[Xuanwo]

So we need to implement ExpressionExecutor in the new planner?

[zhyass]

So we need to implement ExpressionExecutor in the new planner?

ExpressionExecutor is used in many places in storages, so we need to consider how to replace it.
For example:
https://github.com/datafuselabs/databend/blob/main/src/query/storages/fuse/src/operations/mutation/block_filter.rs#L70
https://github.com/datafuselabs/databend/blob/main/src/query/storages/index/src/range_filter.rs#L70

[Xuanwo]

ExpressionExecutor is used in many places in storages, so we need to consider how to replace it.

Yep. This is precisely the problem I met in migrating sqlparser.

Cc @sundy-li @leiysky for advice.

[sundy-li]

There is Evaluator::eval_physical_scalar, maybe we can use it instead?

But in ExpressionExecutor, which codes show that it depends on sqlparser ?

[Xuanwo]

But in ExpressionExecutor, which codes show that it depends on sqlparser ?

Ah, I found I made a mistake here. I will re-read related code tomorrow.