[SPARK-13869][SQL] Remove redundant conditions while combining filters

[sameeragarwal]

What changes were proposed in this pull request?

[I'll link it to the JIRA once ASF JIRA is back online]

This PR modifies the existing CombineFilters rule to remove redundant conditions while combining individual filter predicates. For instance, queries of the form table.where('a === 1 && 'b === 1).where('a === 1 && 'c === 1) will now be optimized to table.where('a === 1 && 'b === 1 && 'c === 1) (instead of table.where('a === 1 && 'a === 1 && 'b === 1 && 'c === 1))

How was this patch tested?

Unit test in FilterPushdownSuite

[sameeragarwal]

cc @yhuai

[SparkQA]

Test build #52973 has finished for PR 11670 at commit e946c1e.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #53092 has finished for PR 11670 at commit f9aba06.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[nongli]

LGTM

How did you decide to do this? Are there other rules that are generating things like this?

[gatorsmile]

LGTM

[nongli]

Can you be more specific? When does this generate a redundant one? Let's make sure we don't continuously generate them and collapse them.

[gatorsmile]

@sameeragarwal I plan to submit a PR for avoiding pushing redundant predicates through the operators, since it is blocking another PR: #11714. Are you doing this?

[sameeragarwal]

I actually found this issue while playing around with some tests. @gatorsmile can you give an example for a case where we generate redundant filters? The code does seem to check the constraints of its children (which are derived from their condition) before generating additional filters.

[gatorsmile]

I thought you are doing this PR for avoiding to push down the redundant inferred/isnotnull conditions. It sounds like my understanding is wrong. As shown in the #11714, the test case failure is caused by this. That is what I plan to fix.

[sameeragarwal]

oh I see, a union -- that was a tricky one! The issue with union is that its constraints are an "intersection" of the constraints of its individual children (which means that our filter inference might push redundant filters to one of more of its child nodes). To fix this, we should make sure that we check the constraints of the children before pushing filters down. Should I make a fix?

[gatorsmile]

I think we need a more general fix for all the predicate push down. I will submit a PR for a general issue instead of fixing Union only.

[sameeragarwal]

yes, I agree.