Repository: spark Updated Branches: refs/heads/master 44c7c62bc -> 34283de16
[SPARK-14839][SQL] Support for other types for `tableProperty` rule in SQL syntax ## What changes were proposed in this pull request? Currently, Scala API supports to take options with the types, `String`, `Long`, `Double` and `Boolean` and Python API also supports other types. This PR corrects `tableProperty` rule to support other types (string, boolean, double and integer) so that support the options for data sources in a consistent way. This will affect other rules such as DBPROPERTIES and TBLPROPERTIES (allowing other types as values). Also, `TODO add bucketing and partitioning.` was removed because it was resolved in https://github.com/apache/spark/commit/24bea000476cdd0b43be5160a76bc5b170ef0b42 ## How was this patch tested? Unit test in `MetastoreDataSourcesSuite.scala`. Author: hyukjinkwon <gurwls...@gmail.com> Closes #13517 from HyukjinKwon/SPARK-14839. Project: http://git-wip-us.apache.org/repos/asf/spark/repo Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/34283de1 Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/34283de1 Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/34283de1 Branch: refs/heads/master Commit: 34283de160808324da02964cd5dc5df80e59ae71 Parents: 44c7c62 Author: hyukjinkwon <gurwls...@gmail.com> Authored: Wed Jul 6 23:57:18 2016 -0400 Committer: Herman van Hovell <hvanhov...@databricks.com> Committed: Wed Jul 6 23:57:18 2016 -0400 ---------------------------------------------------------------------- .../apache/spark/sql/catalyst/parser/SqlBase.g4 | 9 ++- .../spark/sql/execution/SparkSqlParser.scala | 22 +++++-- .../sql/execution/command/DDLCommandSuite.scala | 61 ++++++++++++++++++++ 3 files changed, 87 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/spark/blob/34283de1/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---------------------------------------------------------------------- diff --git a/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 b/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 index 4c15f9c..7ccbb2d 100644 --- a/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 +++ b/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 @@ -249,7 +249,7 @@ tablePropertyList ; tableProperty - : key=tablePropertyKey (EQ? value=STRING)? + : key=tablePropertyKey (EQ? value=tablePropertyValue)? ; tablePropertyKey @@ -257,6 +257,13 @@ tablePropertyKey | STRING ; +tablePropertyValue + : INTEGER_VALUE + | DECIMAL_VALUE + | booleanValue + | STRING + ; + constantList : '(' constant (',' constant)* ')' ; http://git-wip-us.apache.org/repos/asf/spark/blob/34283de1/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala ---------------------------------------------------------------------- diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala index 42ec210..f77801f 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala @@ -311,8 +311,6 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder { /** * Create a [[CreateTableUsing]] or a [[CreateTableUsingAsSelect]] logical plan. - * - * TODO add bucketing and partitioning. */ override def visitCreateTableUsing(ctx: CreateTableUsingContext): LogicalPlan = withOrigin(ctx) { val (table, temp, ifNotExists, external) = visitCreateTableHeader(ctx.createTableHeader) @@ -413,7 +411,7 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder { ctx: TablePropertyListContext): Map[String, String] = withOrigin(ctx) { val properties = ctx.tableProperty.asScala.map { property => val key = visitTablePropertyKey(property.key) - val value = Option(property.value).map(string).orNull + val value = visitTablePropertyValue(property.value) key -> value } // Check for duplicate property names. @@ -426,7 +424,7 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder { */ private def visitPropertyKeyValues(ctx: TablePropertyListContext): Map[String, String] = { val props = visitTablePropertyList(ctx) - val badKeys = props.filter { case (_, v) => v == null }.keys + val badKeys = props.collect { case (key, null) => key } if (badKeys.nonEmpty) { operationNotAllowed( s"Values must be specified for key(s): ${badKeys.mkString("[", ",", "]")}", ctx) @@ -461,6 +459,22 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder { } /** + * A table property value can be String, Integer, Boolean or Decimal. This function extracts + * the property value based on whether its a string, integer, boolean or decimal literal. + */ + override def visitTablePropertyValue(value: TablePropertyValueContext): String = { + if (value == null) { + null + } else if (value.STRING != null) { + string(value.STRING) + } else if (value.booleanValue != null) { + value.getText.toLowerCase + } else { + value.getText + } + } + + /** * Create a [[CreateDatabaseCommand]] command. * * For example: http://git-wip-us.apache.org/repos/asf/spark/blob/34283de1/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLCommandSuite.scala ---------------------------------------------------------------------- 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 e1a7b9b..23c2bef 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 @@ -856,4 +856,65 @@ class DDLCommandSuite extends PlanTest { comparePlans(parsed2, expected2) comparePlans(parsed3, expected3) } + + test("support for other types in DBPROPERTIES") { + val sql = + """ + |CREATE DATABASE database_name + |LOCATION '/home/user/db' + |WITH DBPROPERTIES ('a'=1, 'b'=0.1, 'c'=TRUE) + """.stripMargin + val parsed = parser.parsePlan(sql) + val expected = CreateDatabaseCommand( + "database_name", + ifNotExists = false, + Some("/home/user/db"), + None, + Map("a" -> "1", "b" -> "0.1", "c" -> "true")) + + comparePlans(parsed, expected) + } + + test("support for other types in TBLPROPERTIES") { + val sql = + """ + |ALTER TABLE table_name + |SET TBLPROPERTIES ('a' = 1, 'b' = 0.1, 'c' = TRUE) + """.stripMargin + val parsed = parser.parsePlan(sql) + val expected = AlterTableSetPropertiesCommand( + TableIdentifier("table_name"), + Map("a" -> "1", "b" -> "0.1", "c" -> "true"), + isView = false) + + comparePlans(parsed, expected) + } + + test("support for other types in OPTIONS") { + val sql = + """ + |CREATE TABLE table_name USING json + |OPTIONS (a 1, b 0.1, c TRUE) + """.stripMargin + val expected = CreateTableUsing( + TableIdentifier("table_name"), + None, + "json", + false, + Map("a" -> "1", "b" -> "0.1", "c" -> "true"), + null, + None, + false, + true) + + parser.parsePlan(sql) match { + case ct: CreateTableUsing => + // We can't compare array in `CreateTableUsing` directly, so here we explicitly + // set partitionColumns to `null` and then compare it. + comparePlans(ct.copy(partitionColumns = null), expected) + case other => + fail(s"Expected to parse ${classOf[CreateTableCommand].getClass.getName} from query," + + s"got ${other.getClass.getName}: $sql") + } + } } --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org For additional commands, e-mail: commits-h...@spark.apache.org
