[SPARK-13139][SQL] Create native DDL commands

[viirya]

JIRA: https://issues.apache.org/jira/browse/SPARK-13139

From JIRA: We currently delegate most DDLs directly to Hive, through NativePlaceholder in HiveQl.scala. In Spark 2.0, we want to provide native implementations for DDLs for both SQLContext and HiveContext.

This PR will do the first step to parse DDL commands and create logical commands that encapsulate them. Actual implementations still delegate to HiveNativeCommand now.

[SparkQA]

Test build #50658 has finished for PR 11048 at commit 77252af.

• This patch fails Scala style tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • case class CreateDataBase(
  • case class CreateFunction(

[SparkQA]

Test build #50660 has finished for PR 11048 at commit 2b38d11.

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

[rxin]

Thanks - this looks pretty good as a start. We will need to add many other ddls, including alter/drop table, etc.

[viirya]

@rxin I've added alter table command support. As this command and corresponding change is big, I think we should let this PR only cover these three commands and do other commands in other PRs. How do you think?

[SparkQA]

Test build #50919 has finished for PR 11048 at commit f9c1397.

• This patch fails Scala style tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #50922 has finished for PR 11048 at commit eb1fab7.

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

[SparkQA]

Test build #50965 has finished for PR 11048 at commit 3db2e1d.

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

[rxin]

@viirya yes we can do this incrementally. Let's just create subtasks under https://issues.apache.org/jira/browse/SPARK-13139

[rxin]

cc @hvanhovell

any suggestions on how we can make this file/function more modular? It is getting too long and we are about to add a lot more statements to it.

[hvanhovell]

The main problem is LogicalPlan parsing, which we can split up in command/ddl/query parsing. We could use partial functions (or something like that) to implement the different parts of the parsing logic.

For instance:

abstract class BaseParser(val conf: ParserConf) extends ParserInterface {
  val planParsers: Seq[PlanParser]

  lazy val planParser = planParsers.reduce(_.orElse(_))

  def nodeToPlan(node: ASTNode): LogicalPlan = {
    planParser.applyOrElse(node, throw new NotImplementedError(node.text))
  }
}

abstract class PlanParser extends PartialFunction[ASTNode, LogicalPlan]

case class ExplainCommandParser(base: BaseParser) extends PlanParser {

  override def isDefinedAt(node: ASTNode): Boolean = node.text == "TOK_EXPLAIN"

  override def apply(v1: ASTNode): LogicalPlan = v1.children match {
    case (crtTbl @ Token("TOK_CREATETABLE" | "TOK_QUERY", _)) :: rest =>
      val extended = rest.exists(_.text.toUpperCase == "EXTENDED")
      ExplainCommand(base.nodeToPlan(crtTbl), extended)
  }
}

class SomeParser(conf: ParserConf) extends BaseParser {
  val planParsers: Seq[PlanParser] = Seq(
    ExplainCommandParser(this))
}

[rxin]

btw @viirya can we create a execution.commands package for this?

[hvanhovell]

We really should test the resulting plans here, and not wait for an AnalysisException to be thrown. I know this is a PITA, but it will save us a lot of headaches in the future.

[hvanhovell]

@viirya I have made an initial pass. This PR is large enough as it is, lets not more commands to it.

[SparkQA]

Test build #51356 has finished for PR 11048 at commit 170bd77.

• This patch fails Scala style tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • abstract class BaseParser(val conf: ParserConf) extends ParserInterface with ParserBase
  • trait ParserBase
  • abstract class NativeDDLCommands(val sql: String) extends RunnableCommand
  • case class AlterTableSetProperties(
  • case class AlterTableDropProperties(
  • case class AlterTableCommandParser(base: CatalystQl) extends PlanParser

[andrewor14]

I see, it's because we want the NativeDDLCommands to be in the sql package, where it shouldn't know anything about Hive. In the future these commands will no longer be passed to Hive directly as a string so they shouldn't be "native" anymore. For now, I would create a temporary method in SQLContext:

// In SQLContext.scala
// TODO: remove this once we call specific operations in the catalog instead
protected[sql] def runDDLCommand(text: String): Seq[Row] = {
  throw new UnsupportedOperationException
}

// In HiveContext.scala
protected[sql] override def runDDLCommand(text: String): Seq[Row] = {
  runHiveSql(text).map(Row(_))
}

even though it's temporary I still think it's cleaner than doing it in the catalog.

[andrewor14]

@viirya Thanks for working on this. The overall approach is very reasonable but a few things can be improved:

• many files need to be moved to the new packages
• many unused imports in all the files
• many fields and methods can be marked as private, but are not
• the splitting of commands into 2 files is kind of arbitrary. IIUC all native commands we currently pass to Hive directly today are grouped under ddl.scala, and the rest are grouped under commands.scala. In the future, however, we would like to handle all the DDLs ourselves, so this division won't make sense anymore.
• general lack of comments in this area of the code makes things difficult to follow (not your fault)

Additionally I think the reason why this patch is so big is because we moved around a lot of files. The moving itself can be done in a separate PR, which would reduce the diff significantly and make the patch easier to review.

Addressing all the outstanding comments will likely take a long time. Would you mind that I take this over? I'll be sure to give you credit in the final patch.

[viirya]

@andrewor14 Thanks for reviewing. I don't mind if you want to take this over. Thanks for the credit!