Introduce rewrite tools on Rel.

[scgkiran]

Add GetInputs, Copy, CopyWithExpressionRewrite to Rel

[CLAassistant]

All committers have signed the CLA.

[vbarua]

This is very promising! We have something similar to this in substrait-java in the RelCopyOnWriteVisitor and ExpressionCopyOnWriteVisitor which allows us to implement arbitrary tree rewrites quite easily.

[jacques-n]

Mostly looking good. Few minor proposed changes.

[jacques-n]

...something similar to this in substrait-java...

Lol, why invent when you can just copy useful patterns, right? :D

[zeroshade]

Rather than repeating this over and over, it feels like we could easily write a simple generic helper function, something like:

func (b *baseReadRel) updateFilters(filter, bestEffortFilter expr.Expression) {
    b.filter, b.bestEffortFilter = filter, bestEffortFilter
}

type exprRewriter interface {
    copyExpressions(func(expr.Expression) (expr.Expression, error)) ([]expr.Expression, error)
    updateFilters(filter, bestEffortFilter expr.Expression)
}

func copyRewriteHelper[T exprRewriter](r *T, rewriteFunc func(expr.Expression) (expr.Expression, error)) (*T, error) {
    newExprs, err := r.copyExpressions(rewriteFunc)
    if err != nil {
        return nil, err
    }

    if newExprs == nil {
        return r, nil
    }

    nt := *r
    nt.updateFilters(newExprs[0], newExprs[1])
    return &nt, nil
}

...

func (v *VirtualTableReadRel) CopyWithExpressionRewrite(rewriteFunc func(expr.Expression) (expr.Expression, error), newInputs ...Rel) (Rel, error) {
    return copyRewriteHelper(v, rewriteFunc)
}

[scgkiran]

I hit some issues when I tried this.

1. func copyRewriteHelper[T exprRewriter](r *T, rewriteFunc RewriteFunc) (*T, error)
   - r is a pointer to interface, complains about unresolved copyReference
   - r is a pointer to interface, so nt := *r won't copy the object?
2. func copyRewriteHelper[T exprRewriter](r T, rewriteFunc RewriteFunc) (*T, error)
   - nt := *r this throws error invalid indirect of type T. Looks like we cannot copy object using interface type variable.

[scgkiran]

Below works, but still some code duplication of is there.

type exprRewriter interface {
	copyExpressions(rewriteFunc RewriteFunc) ([]expr.Expression, error)
	getExpressions() []expr.Expression
	updateFilters(filter []expr.Expression)
	copy() Rel
}

func copyRewriteHelper(n exprRewriter, rewriteFunc RewriteFunc) (Rel, error) {
	newExprs, err := n.copyExpressions(rewriteFunc)
	if err != nil {
		return nil, err
	}
	if slices.Equal(newExprs, n.getExpressions()) {
		return n.(Rel), nil
	}
	nt := n.copy()
	nt.(exprRewriter).updateFilters(newExprs)
	return nt, nil
}

func (n *NamedTableReadRel) copy() Rel {
	ntr := *n
	return &ntr
}

func (n *NamedTableReadRel) CopyWithExpressionRewrite1(rewriteFunc RewriteFunc, _ ...Rel) (Rel, error) {
	return copyRewriteHelper(n, rewriteFunc)
}

[zeroshade]

Could we do

join := j.Copy(newInputs...)
join.expr, join.postJoinFilter = exprs[0], exprs[1]

instead?

[scgkiran]

Did this partially. Using j.Copy asks for a type cast to JoinRel.

[jacques-n]

This looks good to me. I see most of the feedback that zeroshade provided has been addressed. I think we should merge this. If the remaining items are not sufficient, let's address in a follow-up ticket.