Fix correctness bug in EffectivePredicateExtractor

[aaneja]

Description

Fixes #23660

During predicate pushdown through a Join, we were

• Pulling up a predicate expression that only refers to constant values
• Incorrectly pushing down this predicate between join sides
• This predicate expression gets optimized to FALSE, but now is pushed down to both sides of the join

This results in the optimizer Incorrectly assuming no results will be produced

Fix

Pulling up expressions happens inEffectivePredicateExtractor#pullExpressionThroughVariables. This was modified to stop pulling up expressions that only refer to constants since this is logically incorrect

Test Plan

• New unit and E2E tests added

Contributor checklist

• Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
• PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
• Documented new properties (with its default value), SQL syntax, functions, or other functionality.
• If release notes are required, they follow the release notes guidelines.
• Adequate tests were added if applicable.
• CI passed.

Release Notes

== NO RELEASE NOTE ==

[elharo]

Nice discovery. Thanks!

[jaystarshot]

Should we add a test from #23660?

[jaystarshot]

Bit confused here, as to why do we have the condition to skip if rewritten has variable references? What if the variable reference is pointing to a constant in some project below?

[jaystarshot]

I guess if it is below, then correctness should already be correct.

[aaneja]

If rewritten has variable references that are in(variables) we do not skip them and pull them up (by adding them to effectiveConjuncts. The problem here is that EqaulityInference is designed to rewrite expressions to just constants (see test), even though, technically this expression does not refer to any variables

However, if the rewritten expression has no variable references, le.g mod(10,5)=1 - this predicate should not be pulled up, to say, a Join node, because it has no references in the outputs of the Join and cannot impact any predicate inferencing there

[aaneja]

@jaystarshot The query used here is logically identical to one from the #23660 issue

[jaystarshot]

But this current only tests execution and not result expectations?

[aaneja]

It asserts that results are identical to those from SELECT regionkey, NULL from region

[aaneja]

If rewritten has variable references that are in(variables) we do not skip them and pull them up (by adding them to effectiveConjuncts. The problem here is that EqaulityInference is designed to rewrite expressions to just constants (see test), even though, technically this expression does not refer to any variables

However, if the rewritten expression has no variable references, le.g mod(10,5)=1 - this predicate should not be pulled up, to say, a Join node, because it has no references in the outputs of the Join and cannot impact any predicate inferencing there