refactor: better ExprRewritable implementation

[chenzl25]

          Actually I'm wondering why this could improve the performance. It appears that a no-op call of `rewrite_exprs_recursive` will still lead to performance overhead of cloning and allocation, which is far from ideal. In this PR, we manually add a visitor dual for a rewriter to avoid calling `rewrite` as much as possible. However, the developers still have to make sure they're identical, just like what we talked in https://github.com/risingwavelabs/risingwave/pull/13587#issuecomment-1822303190.

I'm considering whether it's possible to reduce the overhead of rewrite...

• We already have has_rewritable_expr to reduce the calls. https://github.com/risingwavelabs/risingwave/blob/d37b87bf641655d775f72997a268d6e881541e6d/src/frontend/src/optimizer/plan_node/expr_rewritable.rs#L26-L28 Is it possible to make it specific to different rewriters? This will somehow be similar to https://doc.rust-lang.org/stable/std/error/struct.Request.html#method.would_be_satisfied_by_ref_of.

• When expressions not actually be rewritten, rewrite_exprs should be able to directly return itself without cloning. This can be achieved with Cow. https://github.com/risingwavelabs/risingwave/blob/d37b87bf641655d775f72997a268d6e881541e6d/src/frontend/src/optimizer/plan_node/logical_project.rs#L172-L180

• Similarly when inputs not changed, clone_with_inputs should be able to directly return itself without cloning. This can be achieved with Rc::ptr_eq. https://github.com/risingwavelabs/risingwave/blob/d37b87bf641655d775f72997a268d6e881541e6d/src/frontend/src/optimizer/plan_node/mod.rs#L300

• There is no need to allocate a Vec<ExprImpl> or clone the expression when rewriting them. https://github.com/risingwavelabs/risingwave/blob/d37b87bf641655d775f72997a268d6e881541e6d/src/frontend/src/optimizer/plan_node/generic/project.rs#L55-L63

Take this for an example, a more efficient version should be...

        for expr in self.exprs.iter_mut() {
            *expr = r.rewrite_expr(std::mem::replace(expr, ExprImpl::Dummy));
        }

where no allocation will be done if there's no actual rewriting.

Originally posted by @BugenZhao in https://github.com/risingwavelabs/risingwave/issues/13609#issuecomment-1823899699

[st1page]

          > * When expressions not actually be rewritten, `rewrite_exprs` should be able to directly return itself without cloning. This can be achieved with `Cow`.

https://github.com/risingwavelabs/risingwave/blob/d37b87bf641655d775f72997a268d6e881541e6d/src/frontend/src/optimizer/plan_node/logical_project.rs#L172-L180

I guess we can achieve it with passing the Arc

use std::rc::Rc;
pub trait ExprRewritable {
    fn rewrite_exprs(self: Rc<Self>) -> Rc<dyn ExprRewritable>;
}

struct Proj {
    a: i32
}

impl ExprRewritable for Proj {
    fn rewrite_exprs(self: Rc<Self>) -> Rc<dyn ExprRewritable> {
        if self.a == 1 {
            return Rc::new(Proj{ a: 2});
        } else {
            return self
        }
    }

}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=af96f135c7068c575bd1a0d9493b7ee3

Originally posted by @st1page in https://github.com/risingwavelabs/risingwave/issues/13609#issuecomment-1823921930

[github-actions[bot]]

This issue has been open for 60 days with no activity.

If you think it is still relevant today, and needs to be done in the near future, you can comment to update the status, or just manually remove the no-issue-activity label.

You can also confidently close this issue as not planned to keep our backlog clean. Don't worry if you think the issue is still valuable to continue in the future. It's searchable and can be reopened when it's time. 😄