[SPARK-13891] [SQL] [TEST] Issue Exceptions when Hitting the Max Iteration Limit in Optimizer and Analyzer

[gatorsmile]

What changes were proposed in this pull request?

This PR is to issue an exception in the unit tests of Spark SQL when hitting the max iteration limit. Then, we can catch the infinite loop bugs in Analyzer and Optimizer.

Will submit separate PRs to fix the root cause that trigger these exceptions. Thanks!

cc @marmbrus

How was this patch tested?

Note: this PR will trigger a lot of failures due to the uncaught issues. For example, one of the issues has been found in #11682

[rxin]

should we just change PlanTest?

[gatorsmile]

yeah, a good idea!

[SparkQA]

Test build #53166 has finished for PR 11714 at commit 797aabb.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • abstract class Strategy
  • case class FixedPoint(maxIterations: Int) extends Strategy

[SparkQA]

Test build #53173 has finished for PR 11714 at commit f351362.

• This patch fails to build.
• This patch merges cleanly.
• This patch adds no public classes.

[gatorsmile]

I will not trigger more tests in this PR until the related PR is merged. Thanks!

[rxin]

I just merged #11682. Are there more?

[SparkQA]

Test build #53175 has finished for PR 11714 at commit 1281f36.

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

[SparkQA]

Test build #53176 has finished for PR 11714 at commit 23663fe.

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

[gatorsmile]

@rxin Yeah. There is another issue. In the predicate push down, we should not push down any predicate if the child's Constraints already contain them. Will submit a PR tonight.

[rxin]

Can you bring this up to date? We can disable rules that make this fail. @marmbrus and I talked a bit and thought this is pretty critical to have.

[gatorsmile]

Sure, will do it now.

[gatorsmile]

@rxin @marmbrus This test build will fail for sure.

The first issue is caused by the conflicts of ColumnPruning and PushPredicateThroughProject.

• The related PRs is [SPARK-13919] [SQL] fix column pruning through filter #11828 by @davies Now, I think his version is more favorable to my PR: [SPARK-13919] [SQL] [WIP] Resolving the Conflicts of ColumnPruning and PushPredicateThroughProject #11745

Another issue is caused by Constraints. The related Optimizer rule is InferFiltersFromConstraints:

• Sure, we can disable it and the related test cases.
• To resolve it, I submitted a PR to avoid the Constraints triggered condition push down: [SPARK-13936] [SQL] Avoid Predicate Pushdown Using Constraints in PushPredicateThroughProject #11765
• At the same time, @viirya is resolving the related issue by cleaning the IsNotNull Constraints generation of compound expressions. [SPARK-13995][SQL] Extract correct IsNotNull constraints for Expression #11809 I think this could also avoid exceeding the max num of iterations. Not 100% sure.

[marmbrus]

I guess I would prioritize getting this in so that we make sure we don't continue to introduce conflicting rules. If including #11828 in this PR is the fastest path to that where there is agreement, lets just include that here. If we need to delete InferFiltersFromConstraints that is also okay. We can restore the code in a follow up PR after the problems have been fixed.

[SparkQA]

Test build #53834 has finished for PR 11714 at commit 23a6cfc.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • abstract class Strategy
  • case class FixedPoint(maxIterations: Int) extends Strategy

[gatorsmile]

I see. Sure. Let me know if anything I can help. Thanks!

[srowen]

This is already set by the test framework. MIght be cleaner not to hard code it in just a few places in the tests.

[gatorsmile]

Are you saying SPARK_TESTING can be set by the test framework?

[srowen]

SPARK_TESTING is an env variable while spark.testing is a JVM system property, but yes both are set by the build for you.

[gatorsmile]

Thank you, will make a try.

[SparkQA]

Test build #53936 has finished for PR 11714 at commit 1a59fcf.

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