Skip to content

[SPARK-7012][SQL] Add support for NOT NULL modifier for column definitions on DDLParser #8746

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

[SPARK-7012][SQL] Add support for NOT NULL modifier for column definitions on DDLParser #8746

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5855489
Add support for NOT NULL modified in column definition
Sep 14, 2015
108b5ea
Add test case for support of NOT NULL modified in create table statement
Oct 2, 2015
4fb6802
Modify test case to use withTempPath and changed variable name to not…
Oct 3, 2015
f68a053
Additional cosmetic changes
Oct 3, 2015
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
19 changes: 11 additions & 8 deletions sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/DDLParser.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ class DDLParser(parseQuery: String => LogicalPlan)
protected val AS = Keyword("AS")
protected val COMMENT = Keyword("COMMENT")
protected val REFRESH = Keyword("REFRESH")
protected val NULL = Keyword("NULL")

protected lazy val ddl: Parser[LogicalPlan] = createTable | describeTable | refreshTable

Expand Down Expand Up @@ -173,13 +174,15 @@ class DDLParser(parseQuery: String => LogicalPlan)
optionName ~ stringLit ^^ { case k ~ v => (k, v) }

protected lazy val column: Parser[StructField] =
ident ~ dataType ~ (COMMENT ~> stringLit).? ^^ { case columnName ~ typ ~ cm =>
val meta = cm match {
case Some(comment) =>
new MetadataBuilder().putString(COMMENT.str.toLowerCase, comment).build()
case None => Metadata.empty
}

StructField(columnName, typ, nullable = true, meta)
ident ~ dataType ~ (NOT ~ NULL).? ~ (COMMENT ~> stringLit).? ^^ {
case columnName ~ typ ~ notNull ~ cm =>
val meta = cm match {
case Some(comment) =>
new MetadataBuilder().putString(COMMENT.str.toLowerCase, comment).build()
case None => Metadata.empty
}

val isNullable = notNull.isEmpty
StructField(columnName, typ, nullable = isNullable, meta)
}
}
19 changes: 19 additions & 0 deletions sql/core/src/test/scala/org/apache/spark/sql/sources/DDLTestSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,4 +113,23 @@ class DDLTestSuite extends DataSourceTest with SharedSQLContext {
assert(attributes.map(_.name) === Seq("col_name", "data_type", "comment"))
assert(attributes.map(_.dataType).toSet === Set(StringType))
}

test("SPARK-7012 Create table statement should support NOT NULL modifier for columns") {
withTempPath { dir =>
val path = dir.getCanonicalPath
sql(
s"""
|CREATE TEMPORARY TABLE tempTableDDL
|( tCol1 INT NOT NULL,
| tCol2 STRING
|)
|USING parquet
|OPTIONS (
| path '$path'
|)
""".stripMargin
)
caseInsensitiveContext.dropTempTable("tempTableDDL")
Copy link
Contributor

@cloud-fan cloud-fan Oct 14, 2015

Choose a reason for hiding this comment

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

improve this test case to make sure we really make that column not nullable, i.e. read the table out and check its schema.

Copy link
Contributor

@smola smola Nov 17, 2015

Choose a reason for hiding this comment

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

@sabhyankar Do you have time to modify this? I think that once the test is improved with @cloud-fan comment, the change is probably good to merge.

Copy link
Author

@sabhyankar sabhyankar Nov 17, 2015

Choose a reason for hiding this comment

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

Hi @smola @cloud-fan - I will take a look at the test case this week and update you guys

}
}
}