Skip to content

[SPARK-15676] [SQL] Disallow Column Names as Partition Columns For Hive Tables #13415

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
gatorsmile wants to merge 4 commits into from
Closed

[SPARK-15676] [SQL] Disallow Column Names as Partition Columns For Hive Tables #13415

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
48ddb08
fix
gatorsmile May 31, 2016
6c5c2d9
address comments.
gatorsmile May 31, 2016
942366f
address comments.
gatorsmile Jun 1, 2016
f4207e3
address comments.
gatorsmile Jun 3, 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
8 changes: 7 additions & 1 deletion sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -937,7 +937,13 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder {

selectQuery match {
case Some(q) => CreateTableAsSelectLogicalPlan(tableDesc, q, ifNotExists)
Copy link
Member Author

@gatorsmile gatorsmile May 31, 2016

Choose a reason for hiding this comment

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

For CTAS, another PR (#13395) resolves the issue by disallowing users to specify Partitioned By clauses.

case None => CreateTableCommand(tableDesc, ifNotExists)
case None =>
val partitionColsInTable = partitionCols.map(_.name).toSet.intersect(cols.map(_.name).toSet)
if (partitionColsInTable.nonEmpty) {
throw new ParseException(s"Column repeated in partitioning columns: " +
partitionColsInTable.mkString("[", ",", "]"), ctx)
Copy link
Contributor

@andrewor14 andrewor14 May 31, 2016

Choose a reason for hiding this comment

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

Hi @gatorsmile, this looks OK but it seems a better place to do it is up there in L885, where we just concatenate the schema with the partition columns together. There we can just check if schema.map(_.name) has any duplicate values.

Copy link
Member Author

@gatorsmile gatorsmile May 31, 2016

Choose a reason for hiding this comment

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

I see. I can move it there.

The reason why I put here is because CTAS should not see the partitioning columns. If we move there, we could issue this error message before the expected message: https://github.com/yhuai/spark/blob/fa8908122a238d6cdc0a9fc0f003221ef5601565/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala#L940-L948

Copy link
Contributor

@andrewor14 andrewor14 May 31, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

that's fine. I would still move it. Maybe I would even move the datasource partition check before this exception; we don't have to throw that one so late.

}
CreateTableCommand(tableDesc, ifNotExists)
}
}

Expand Down
6 changes: 6 additions & 0 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 @@ -319,6 +319,12 @@ class DDLCommandSuite extends PlanTest {
assert(ct.table.storage.locationUri == Some("/something/anything"))
}

test("create table - column repeated in partitioning columns") {
val query = "CREATE TABLE tab1 (key INT, value STRING) PARTITIONED BY (key INT, hr STRING)"
val e = intercept[ParseException] { parser.parsePlan(query) }
assert(e.getMessage.contains("Column repeated in partitioning columns: [key]"))
}

test("create table using - with partitioned by") {
val query = "CREATE TABLE my_tab(a INT, b STRING) USING parquet PARTITIONED BY (a)"
val expected = CreateTableUsing(
Expand Down