Skip to content

[SPARK-13139][SQL] Follow-ups to #11573 #11667

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
andrewor14 wants to merge 2 commits into from
Closed

[SPARK-13139][SQL] Follow-ups to #11573 #11667

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
687f011
CREATE FUNCTION handle multiple jars
Mar 11, 2016
8ed596d
Update comments, add more examples
Mar 12, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 37 additions & 15 deletions sql/core/src/main/scala/org/apache/spark/sql/execution/SparkQl.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,18 @@ 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).
*
Copy link
Contributor

@yhuai yhuai Mar 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not very related to this PR. @hvanhovell Do you know when we will have something like

TOK_TABLEPROPERTY
  'key_part1'
  'key_part2'
  'key_part3'
  'value'

(the form that matches the original comment extract properties in the form of a list ['key1', 'key2', 'key3', 'value'] into a pair (key1.key2.key3, value).)

Copy link
Contributor

@hvanhovell hvanhovell Mar 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That happens when we have a dot-separated key, e.g.: a.b.c The parser identifies each part of the identifier separately, e.g: a, b & c. It is a bit of a PITA to re-construct this in ANTLR3, so we currently do this in the tree parser.

Copy link
Contributor

@yhuai yhuai Mar 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrewor14 I will change For each node, extract properties in the form of a list ['key1', 'key2', 'key3', 'value'] into a pair (key1.key2.key3, value). to 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).. Hopefully, this will make the comment easier to understand.

Copy link
Contributor

@yhuai yhuai Mar 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hvanhovell Thanks!

Copy link
Contributor Author

@andrewor14 andrewor14 Mar 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sg

* Example format:
*
* TOK_TABLEPROPERTY
* :- 'k1'
* +- 'v1'
* TOK_TABLEPROPERTY
* :- 'k2'
* +- 'v2'
* TOK_TABLEPROPERTY
* :- 'k3'
* +- 'v3'
*/
private def extractProps(
props: Seq[ASTNode],
Expand Down Expand Up @@ -101,6 +113,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
Expand All @@ -112,16 +134,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
Expand All @@ -139,9 +161,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"
Expand All @@ -153,8 +175,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)
Expand Down
93 changes: 48 additions & 45 deletions sql/core/src/main/scala/org/apache/spark/sql/execution/command/AlterTableCommandParser.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) =>
Expand All @@ -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 {
Expand Down Expand Up @@ -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) =>
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
10 changes: 5 additions & 5 deletions sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLCommandSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down