From 9a1680c2c85da4096bad71743debb2ccacdfd79f Mon Sep 17 00:00:00 2001 From: Andrew Or Date: Mon, 14 Mar 2016 09:57:24 -0700 Subject: [SPARK-13139][SQL] Follow-ups to #11573 Addressing outstanding comments in #11573. Jenkins, new test case in `DDLCommandSuite` Author: Andrew Or Closes #11667 from andrewor14/ddl-parser-followups. --- .../org/apache/spark/sql/execution/SparkQl.scala | 57 +++++++++---- .../command/AlterTableCommandParser.scala | 93 +++++++++++----------- .../apache/spark/sql/execution/command/ddl.scala | 2 +- .../sql/execution/command/DDLCommandSuite.scala | 10 +-- 4 files changed, 94 insertions(+), 68 deletions(-) (limited to 'sql/core') diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkQl.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkQl.scala index d12dab567b..8dde308f96 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkQl.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkQl.scala @@ -34,8 +34,21 @@ private[sql] class SparkQl(conf: ParserConf = SimpleParserConf()) extends Cataly } /** - * For each node, extract properties in the form of a list ['key1', 'key2', 'key3', 'value'] - * into a pair (key1.key2.key3, value). + * For each node, extract properties in the form of a list + * ['key_part1', 'key_part2', 'key_part3', 'value'] + * into a pair (key_part1.key_part2.key_part3, value). + * + * Example format: + * + * TOK_TABLEPROPERTY + * :- 'k1' + * +- 'v1' + * TOK_TABLEPROPERTY + * :- 'k2' + * +- 'v2' + * TOK_TABLEPROPERTY + * :- 'k3' + * +- 'v3' */ private def extractProps( props: Seq[ASTNode], @@ -101,6 +114,16 @@ private[sql] class SparkQl(conf: ParserConf = SimpleParserConf()) extends Cataly } val props = dbprops.toSeq.flatMap { case Token("TOK_DATABASEPROPERTIES", Token("TOK_DBPROPLIST", propList) :: Nil) => + // Example format: + // + // TOK_DATABASEPROPERTIES + // +- TOK_DBPROPLIST + // :- TOK_TABLEPROPERTY + // : :- 'k1' + // : +- 'v1' + // :- TOK_TABLEPROPERTY + // :- 'k2' + // +- 'v2' extractProps(propList, "TOK_TABLEPROPERTY") case _ => parseFailed("Invalid CREATE DATABASE command", node) }.toMap @@ -112,16 +135,16 @@ private[sql] class SparkQl(conf: ParserConf = SimpleParserConf()) extends Cataly // Example format: // // TOK_CREATEFUNCTION - // :- db_name - // :- func_name - // :- alias - // +- TOK_RESOURCE_LIST - // :- TOK_RESOURCE_URI - // : :- TOK_JAR - // : +- '/path/to/jar' - // +- TOK_RESOURCE_URI - // :- TOK_FILE - // +- 'path/to/file' + // :- db_name + // :- func_name + // :- alias + // +- TOK_RESOURCE_LIST + // :- TOK_RESOURCE_URI + // : :- TOK_JAR + // : +- '/path/to/jar' + // +- TOK_RESOURCE_URI + // :- TOK_FILE + // +- 'path/to/file' val (funcNameArgs, otherArgs) = args.partition { case Token("TOK_RESOURCE_LIST", _) => false case Token("TOK_TEMPORARY", _) => false @@ -139,9 +162,9 @@ private[sql] class SparkQl(conf: ParserConf = SimpleParserConf()) extends Cataly } // Extract other keywords, if they exist val Seq(rList, temp) = getClauses(Seq("TOK_RESOURCE_LIST", "TOK_TEMPORARY"), otherArgs) - val resourcesMap = rList.toSeq.flatMap { - case Token("TOK_RESOURCE_LIST", resources) => - resources.map { + val resources: Seq[(String, String)] = rList.toSeq.flatMap { + case Token("TOK_RESOURCE_LIST", resList) => + resList.map { case Token("TOK_RESOURCE_URI", rType :: Token(rPath, Nil) :: Nil) => val resourceType = rType match { case Token("TOK_JAR", Nil) => "jar" @@ -153,8 +176,8 @@ private[sql] class SparkQl(conf: ParserConf = SimpleParserConf()) extends Cataly case _ => parseFailed("Invalid CREATE FUNCTION command", node) } case _ => parseFailed("Invalid CREATE FUNCTION command", node) - }.toMap - CreateFunction(funcName, alias, resourcesMap, temp.isDefined)(node.source) + } + CreateFunction(funcName, alias, resources, temp.isDefined)(node.source) case Token("TOK_ALTERTABLE", alterTableArgs) => AlterTableCommandParser.parse(node) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/AlterTableCommandParser.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/AlterTableCommandParser.scala index 58639275c1..9fbe6db467 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/AlterTableCommandParser.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/AlterTableCommandParser.scala @@ -55,20 +55,22 @@ object AlterTableCommandParser { /** * Extract partition spec from the given [[ASTNode]] as a map, assuming it exists. * - * Expected format: - * +- TOK_PARTSPEC - * :- TOK_PARTVAL - * : :- dt - * : +- '2008-08-08' - * +- TOK_PARTVAL - * :- country - * +- 'us' + * Example format: + * + * TOK_PARTSPEC + * :- TOK_PARTVAL + * : :- dt + * : +- '2008-08-08' + * +- TOK_PARTVAL + * :- country + * +- 'us' */ private def parsePartitionSpec(node: ASTNode): Map[String, String] = { node match { case Token("TOK_PARTSPEC", partitions) => partitions.map { // Note: sometimes there's a "=", "<" or ">" between the key and the value + // (e.g. when dropping all partitions with value > than a certain constant) case Token("TOK_PARTVAL", ident :: conj :: constant :: Nil) => (cleanAndUnquoteString(ident.text), cleanAndUnquoteString(constant.text)) case Token("TOK_PARTVAL", ident :: constant :: Nil) => @@ -86,15 +88,16 @@ object AlterTableCommandParser { /** * Extract table properties from the given [[ASTNode]] as a map, assuming it exists. * - * Expected format: - * +- TOK_TABLEPROPERTIES - * +- TOK_TABLEPROPLIST - * :- TOK_TABLEPROPERTY - * : :- 'test' - * : +- 'value' - * +- TOK_TABLEPROPERTY - * :- 'comment' - * +- 'new_comment' + * Example format: + * + * TOK_TABLEPROPERTIES + * +- TOK_TABLEPROPLIST + * :- TOK_TABLEPROPERTY + * : :- 'test' + * : +- 'value' + * +- TOK_TABLEPROPERTY + * :- 'comment' + * +- 'new_comment' */ private def extractTableProps(node: ASTNode): Map[String, String] = { node match { @@ -209,21 +212,21 @@ object AlterTableCommandParser { Token("TOK_TABCOLNAME", colNames) :: colValues :: rest) :: Nil) :: _ => // Example format: // - // +- TOK_ALTERTABLE_SKEWED - // :- TOK_TABLESKEWED - // : :- TOK_TABCOLNAME - // : : :- dt - // : : +- country - // :- TOK_TABCOLVALUE_PAIR - // : :- TOK_TABCOLVALUES - // : : :- TOK_TABCOLVALUE - // : : : :- '2008-08-08' - // : : : +- 'us' - // : :- TOK_TABCOLVALUES - // : : :- TOK_TABCOLVALUE - // : : : :- '2009-09-09' - // : : : +- 'uk' - // +- TOK_STOREASDIR + // TOK_ALTERTABLE_SKEWED + // :- TOK_TABLESKEWED + // : :- TOK_TABCOLNAME + // : : :- dt + // : : +- country + // :- TOK_TABCOLVALUE_PAIR + // : :- TOK_TABCOLVALUES + // : : :- TOK_TABCOLVALUE + // : : : :- '2008-08-08' + // : : : +- 'us' + // : :- TOK_TABCOLVALUES + // : : :- TOK_TABCOLVALUE + // : : : :- '2009-09-09' + // : : : +- 'uk' + // +- TOK_STOREASDIR val names = colNames.map { n => cleanAndUnquoteString(n.text) } val values = colValues match { case Token("TOK_TABCOLVALUE", vals) => @@ -260,20 +263,20 @@ object AlterTableCommandParser { case Token("TOK_ALTERTABLE_SKEWED_LOCATION", Token("TOK_SKEWED_LOCATIONS", Token("TOK_SKEWED_LOCATION_LIST", locationMaps) :: Nil) :: Nil) :: _ => - // Expected format: + // Example format: // - // +- TOK_ALTERTABLE_SKEWED_LOCATION - // +- TOK_SKEWED_LOCATIONS - // +- TOK_SKEWED_LOCATION_LIST - // :- TOK_SKEWED_LOCATION_MAP - // : :- 'col1' - // : +- 'loc1' - // +- TOK_SKEWED_LOCATION_MAP - // :- TOK_TABCOLVALUES - // : +- TOK_TABCOLVALUE - // : :- 'col2' - // : +- 'col3' - // +- 'loc2' + // TOK_ALTERTABLE_SKEWED_LOCATION + // +- TOK_SKEWED_LOCATIONS + // +- TOK_SKEWED_LOCATION_LIST + // :- TOK_SKEWED_LOCATION_MAP + // : :- 'col1' + // : +- 'loc1' + // +- TOK_SKEWED_LOCATION_MAP + // :- TOK_TABCOLVALUES + // : +- TOK_TABCOLVALUE + // : :- 'col2' + // : +- 'col3' + // +- 'loc2' val skewedMaps = locationMaps.flatMap { case Token("TOK_SKEWED_LOCATION_MAP", col :: loc :: Nil) => col match { diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala index 9df58d214a..3fb2e34101 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala @@ -55,7 +55,7 @@ case class CreateDatabase( case class CreateFunction( functionName: String, alias: String, - resourcesMap: Map[String, String], + resources: Seq[(String, String)], isTemp: Boolean)(sql: String) extends NativeDDLCommand(sql) with Logging diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLCommandSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLCommandSuite.scala index 0d632a8a13..6f1eea273f 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLCommandSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLCommandSuite.scala @@ -48,26 +48,26 @@ class DDLCommandSuite extends PlanTest { val sql1 = """ |CREATE TEMPORARY FUNCTION helloworld as - |'com.matthewrathbone.example.SimpleUDFExample' USING JAR '/path/to/jar', - |FILE 'path/to/file' + |'com.matthewrathbone.example.SimpleUDFExample' USING JAR '/path/to/jar1', + |JAR '/path/to/jar2' """.stripMargin val sql2 = """ |CREATE FUNCTION hello.world as |'com.matthewrathbone.example.SimpleUDFExample' USING ARCHIVE '/path/to/archive', - |FILE 'path/to/file' + |FILE '/path/to/file' """.stripMargin val parsed1 = parser.parsePlan(sql1) val parsed2 = parser.parsePlan(sql2) val expected1 = CreateFunction( "helloworld", "com.matthewrathbone.example.SimpleUDFExample", - Map("jar" -> "/path/to/jar", "file" -> "path/to/file"), + Seq(("jar", "/path/to/jar1"), ("jar", "/path/to/jar2")), isTemp = true)(sql1) val expected2 = CreateFunction( "hello.world", "com.matthewrathbone.example.SimpleUDFExample", - Map("archive" -> "/path/to/archive", "file" -> "path/to/file"), + Seq(("archive", "/path/to/archive"), ("file", "/path/to/file")), isTemp = false)(sql2) comparePlans(parsed1, expected1) comparePlans(parsed2, expected2) -- cgit v1.2.3