Skip to content

Commit

Permalink
[SPARK-20412] Throw ParseException from visitNonOptionalPartitionSpec…
Browse files Browse the repository at this point in the history
… instead of returning null values.

## What changes were proposed in this pull request?

If a partitionSpec is supposed to not contain optional values, a ParseException should be thrown, and not nulls returned.
The nulls can later cause NullPointerExceptions in places not expecting them.

## How was this patch tested?

A query like "SHOW PARTITIONS tbl PARTITION(col1='val1', col2)" used to throw a NullPointerException.
Now it throws a ParseException.

Author: Juliusz Sompolski <julek@databricks.com>

Closes #17707 from juliuszsompolski/SPARK-20412.
  • Loading branch information
juliuszsompolski authored and cloud-fan committed Apr 21, 2017
1 parent 3476799 commit c9e6035
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,10 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with Logging {
*/
protected def visitNonOptionalPartitionSpec(
ctx: PartitionSpecContext): Map[String, String] = withOrigin(ctx) {
visitPartitionSpec(ctx).mapValues(_.orNull).map(identity)
visitPartitionSpec(ctx).map {
case (key, None) => throw new ParseException(s"Found an empty partition key '$key'.", ctx)
case (key, Some(value)) => key -> value
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -530,13 +530,13 @@ class DDLCommandSuite extends PlanTest {
""".stripMargin
val sql4 =
"""
|ALTER TABLE table_name PARTITION (test, dt='2008-08-08',
|ALTER TABLE table_name PARTITION (test=1, dt='2008-08-08',
|country='us') SET SERDE 'org.apache.class' WITH SERDEPROPERTIES ('columns'='foo,bar',
|'field.delim' = ',')
""".stripMargin
val sql5 =
"""
|ALTER TABLE table_name PARTITION (test, dt='2008-08-08',
|ALTER TABLE table_name PARTITION (test=1, dt='2008-08-08',
|country='us') SET SERDEPROPERTIES ('columns'='foo,bar', 'field.delim' = ',')
""".stripMargin
val parsed1 = parser.parsePlan(sql1)
Expand All @@ -558,12 +558,12 @@ class DDLCommandSuite extends PlanTest {
tableIdent,
Some("org.apache.class"),
Some(Map("columns" -> "foo,bar", "field.delim" -> ",")),
Some(Map("test" -> null, "dt" -> "2008-08-08", "country" -> "us")))
Some(Map("test" -> "1", "dt" -> "2008-08-08", "country" -> "us")))
val expected5 = AlterTableSerDePropertiesCommand(
tableIdent,
None,
Some(Map("columns" -> "foo,bar", "field.delim" -> ",")),
Some(Map("test" -> null, "dt" -> "2008-08-08", "country" -> "us")))
Some(Map("test" -> "1", "dt" -> "2008-08-08", "country" -> "us")))
comparePlans(parsed1, expected1)
comparePlans(parsed2, expected2)
comparePlans(parsed3, expected3)
Expand Down Expand Up @@ -832,6 +832,14 @@ class DDLCommandSuite extends PlanTest {
assert(e.contains("Found duplicate keys 'a'"))
}

test("empty values in non-optional partition specs") {
val e = intercept[ParseException] {
parser.parsePlan(
"SHOW PARTITIONS dbx.tab1 PARTITION (a='1', b)")
}.getMessage
assert(e.contains("Found an empty partition key 'b'"))
}

test("drop table") {
val tableName1 = "db.tab"
val tableName2 = "tab"
Expand Down

0 comments on commit c9e6035

Please sign in to comment.