[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 24bea00047
## How was this patch tested?
Unit test in `MetastoreDataSourcesSuite.scala`.
Author: hyukjinkwon <gurwls223@gmail.com>
Closes #13517 from HyukjinKwon/SPARK-14839.
This commit is contained in:
parent
44c7c62bcf
commit
34283de160
|
@ -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)* ')'
|
||||
;
|
||||
|
|
|
@ -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)
|
||||
|
@ -460,6 +458,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.
|
||||
*
|
||||
|
|
|
@ -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")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue