Remove ExpressionEquivalence

[martint]

This abstraction doesn't appear to be needed anymore.

Fixes #21235

Release notes

(x) Release notes are required, with the following suggested text:

# Section
* TBD. ({issue}`21235`)

[findepi]

Remove TestExpressionEquivalence

Did you mean "TestEE" or "EE"?

[martint]

Good catch. Yes, ExpressionEquivalence and associated tests.

[findepi]

so the assumption is that newJoinPredicate cannot be similar to (equivalent), but different from, joinPredicate?

what does guarantee this?

[martint]

This is just a stronger requirement to detect whether the predicate was rewritten by the optimization. Before, it just checked whether they were equivalent based on some semantic-preserving transformations. Now it's based on whether they are exactly the same.

[sopel39]

The question is, was ExpressionEquivalence more forgiving than equals? Maybe we keep ExpressionEquivalence abstraction but with equals inside?

[martint]

Yes, it was more lenient than equality. However, I don't think that matters in practice.

The only canonicalizations this class supports are:

• AND(AND(x, y), z) => AND(x, y, z). Same for OR.
• AND(x, x, x, a) => AND(x, a). Same for OR. I believe there's a bug here, though, as it ignores non-deterministic operations that should be evaluated multiple times.
• a = b => b = a
• a is distinct from b => b is distinct from a

All of these are already handled by the expression optimizer and canonicalizer.

[martint]

I believe there's a bug here, though, as it ignores non-deterministic operations that should be evaluated multiple times.

This demonstrates the issue:

ResolvedFunction random = new TestingFunctionResolution().resolveFunction("random", fromTypes());
Expression expression = new IsNull(new Call(random, List.of()));
assertNotEquivalent(
        new Logical(AND, List.of(expression, expression)),
        expression);

[findepi]

All of these are already handled by the expression optimizer and canonicalizer.

the newJoinPredicate comes thru the optimizer (PredicatePushDown.Rewriter#simplifyExpression), does it also do canonicalization?

[findepi]

LGTM, but i don't think I know why/when it became obsolete.

@sopel39 do you want to take a look?

[martint]

LGTM, but i don't think I know why/when it became obsolete.

The optimizer performs many of the canonicalizations that this class was doing. I wasn't able to find any queries that produce different plans with or without this code.