[SPARK-9293] [SPARK-9813] Analysis should check that set operations are only performed on tables with equal numbers of columns

[JoshRosen]

This patch adds an analyzer rule to ensure that set operations (union, intersect, and except) are only applied to tables with the same number of columns. Without this rule, there are scenarios where invalid queries can return incorrect results instead of failing with error messages; SPARK-9813 provides one example of this problem. In other cases, the invalid query can crash at runtime with extremely confusing exceptions.

I also performed a bit of cleanup to refactor some of those logical operators' code into a common SetOperation base class.

[SparkQA]

Test build #38297 has finished for PR 7631 at commit 326d759.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • case class ChangePrecision(child: Expression) extends UnaryExpression
  • abstract class SetOperation(left: LogicalPlan, right: LogicalPlan) extends BinaryNode
  • case class Union(left: LogicalPlan, right: LogicalPlan) extends SetOperation(left, right)
  • case class Intersect(left: LogicalPlan, right: LogicalPlan) extends SetOperation(left, right)
  • case class Except(left: LogicalPlan, right: LogicalPlan) extends SetOperation(left, right)
  • case class DecimalType(precision: Int, scale: Int) extends FractionalType
  • case class DecimalConversion(precision: Int, scale: Int) extends JDBCConversion

[JoshRosen]

Huh, it looks like this failed some Hive tests due to the new assertion that I added.

[cloud-fan]

Someone has tried to add this restrict before in #6174, but failed as hive support different length.

[JoshRosen]

My patch for this was motivated by the fact that my fuzz tester was throwing runtime errors for queries involving UNION ALL with differing numbers of columns: if you run such a UNION ALL query and then attempt to convert the result to a SchemaRDD you can get ArrayIndexOutOfBounds exceptions in CatalystTypeConverters. I'll see if there's a better way to fix this issue.

[rxin]

@cloud-fan what does Hive do? null for all the missing columns?

[cloud-fan]

I tried it locally, hive will report error if will union 2 select with different output length. But in our test we union 2 InsertIntoTable operations which seems a special case for hive(or maybe the output of InsertIntoTable is Nil in hive?).

[SparkQA]

Test build #40999 has finished for PR 7631 at commit b92d817.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • abstract class SetOperation(left: LogicalPlan, right: LogicalPlan) extends BinaryNode
  • case class Union(left: LogicalPlan, right: LogicalPlan) extends SetOperation(left, right)
  • case class Intersect(left: LogicalPlan, right: LogicalPlan) extends SetOperation(left, right)
  • case class Except(left: LogicalPlan, right: LogicalPlan) extends SetOperation(left, right)

[marmbrus]

There is no particular reason that InsertIntoTable should have the same schema as its child. Initially I was thinking that you might want to chain the insert with other operations. However, [the current implementation(https://github.com/apache/spark/blob/master/sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/InsertIntoHiveTable.scala#L257) doesn't take advantage of this possibility so I'd propose we just make its output empty so that we can merge this patch.

[SparkQA]

Test build #41413 has finished for PR 7631 at commit a0bd50c.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • abstract class SetOperation(left: LogicalPlan, right: LogicalPlan) extends BinaryNode
  • case class Union(left: LogicalPlan, right: LogicalPlan) extends SetOperation(left, right)
  • case class Intersect(left: LogicalPlan, right: LogicalPlan) extends SetOperation(left, right)
  • case class Except(left: LogicalPlan, right: LogicalPlan) extends SetOperation(left, right)

[SparkQA]

Test build #41414 has finished for PR 7631 at commit 362e749.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • abstract class SetOperation(left: LogicalPlan, right: LogicalPlan) extends BinaryNode
  • case class Union(left: LogicalPlan, right: LogicalPlan) extends SetOperation(left, right)
  • case class Intersect(left: LogicalPlan, right: LogicalPlan) extends SetOperation(left, right)
  • case class Except(left: LogicalPlan, right: LogicalPlan) extends SetOperation(left, right)

[JoshRosen]

@marmbrus, I've brought this up to date based on your suggestion above, so do you mind taking a look to check whether I've handled the output correctly here?

[cloud-fan]

LGTM

[marmbrus]

LGTM

[marmbrus]

Merging into master and branch-1.5