Skip to content

[SPARK-17518] [SQL] Block Users to Specify the Internal Data Source Provider Hive #15073

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 7 commits into from
Closed

[SPARK-17518] [SQL] Block Users to Specify the Internal Data Source Provider Hive #15073

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ef0fe2e
block users to provide the format hive.
gatorsmile Sep 13, 2016
f2366ba
fix.
gatorsmile Sep 13, 2016
38e958b
Merge remote-tracking branch 'upstream/master' into formatHive
gatorsmile Sep 14, 2016
9711edb
fix test cases
gatorsmile Sep 14, 2016
44f335b
address comments
gatorsmile Sep 16, 2016
da5bec6
address comments.
gatorsmile Sep 17, 2016
ef174c1
address comments
gatorsmile Sep 17, 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
3 changes: 3 additions & 0 deletions sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,9 @@ final class DataFrameWriter[T] private[sql](ds: Dataset[T]) {
}

private def saveAsTable(tableIdent: TableIdentifier): Unit = {
if (source.toLowerCase == "hive") {
throw new AnalysisException("Cannot create hive serde table with saveAsTable API")
}

val tableExists = df.sparkSession.sessionState.catalog.tableExists(tableIdent)

Expand Down
5 changes: 4 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 @@ -22,7 +22,7 @@ import scala.collection.JavaConverters._
import org.antlr.v4.runtime.{ParserRuleContext, Token}
import org.antlr.v4.runtime.tree.TerminalNode

import org.apache.spark.sql.SaveMode
import org.apache.spark.sql.{AnalysisException, SaveMode}
import org.apache.spark.sql.catalyst.{FunctionIdentifier, TableIdentifier}
import org.apache.spark.sql.catalyst.catalog._
import org.apache.spark.sql.catalyst.parser._
Expand Down Expand Up @@ -316,6 +316,9 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder {
}
val options = Option(ctx.tablePropertyList).map(visitPropertyKeyValues).getOrElse(Map.empty)
val provider = ctx.tableProvider.qualifiedName.getText
if (provider.toLowerCase == "hive") {
throw new AnalysisException("Cannot create hive serde table with CREATE TABLE USING")
}
val schema = Option(ctx.colTypeList()).map(createStructType)
val partitionColumnNames =
Option(ctx.partitionColumnNames)
Expand Down
2 changes: 1 addition & 1 deletion sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ class CatalogImpl(sparkSession: SparkSession) extends Catalog {
source: String,
schema: StructType,
options: Map[String, String]): DataFrame = {
if (source == "hive") {
if (source.toLowerCase == "hive") {
throw new AnalysisException("Cannot create hive serde table with createExternalTable API.")
}

Expand Down
7 changes: 7 additions & 0 deletions sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,13 @@ class CatalogSuite
assert(e2.message == "Cannot create a file-based external data source table without path")
}

test("createExternalTable should fail if provider is hive") {
val e = intercept[AnalysisException] {
spark.catalog.createExternalTable("tbl", "HiVe", Map.empty[String, String])
}
assert(e.message.contains("Cannot create hive serde table with createExternalTable API"))
}

// TODO: add tests for the rest of them

}
2 changes: 1 addition & 1 deletion sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveStrategies.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ private[hive] trait HiveStrategies {
// `ErrorIfExists` mode, and `DataFrameWriter.saveAsTable` doesn't support hive serde
// tables yet.
if (mode == SaveMode.Append || mode == SaveMode.Overwrite) {
throw new AnalysisException("" +
throw new AnalysisException(
"CTAS for hive serde tables does not support append or overwrite semantics.")
}

Expand Down
51 changes: 51 additions & 0 deletions sql/hive/src/test/scala/org/apache/spark/sql/hive/MetastoreDataSourcesSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import org.apache.hadoop.fs.Path
import org.apache.spark.sql._
import org.apache.spark.sql.catalyst.TableIdentifier
import org.apache.spark.sql.catalyst.catalog.{CatalogStorageFormat, CatalogTable, CatalogTableType}
import org.apache.spark.sql.catalyst.parser.ParseException
import org.apache.spark.sql.execution.datasources.{HadoopFsRelation, LogicalRelation}
import org.apache.spark.sql.hive.HiveExternalCatalog._
import org.apache.spark.sql.hive.client.HiveClient
Expand Down Expand Up @@ -1151,6 +1152,56 @@ class MetastoreDataSourcesSuite extends QueryTest with SQLTestUtils with TestHiv
}
}

test("save API - format hive") {
Copy link
Contributor

@cloud-fan cloud-fan Sep 15, 2016

Choose a reason for hiding this comment

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

For this API, previously we will fail with message Failed to find data source: hive right? Should we change it?

Copy link
Member Author

@gatorsmile gatorsmile Sep 16, 2016

Choose a reason for hiding this comment

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

Sure, change all of them to the message Failed to find data source: hive

Copy link
Contributor

@cloud-fan cloud-fan Sep 16, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Oh sorry I missed this one, what I was asking is, we should only check the provider in saveAsTable, so that the save API is totally untouched.

Copy link
Member Author

@gatorsmile gatorsmile Sep 17, 2016

Choose a reason for hiding this comment

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

uh... I see.

withTempDir { dir =>
val path = dir.getCanonicalPath
val e = intercept[ClassNotFoundException] {
spark.range(10).write.format("hive").mode(SaveMode.Ignore).save(path)
}.getMessage
assert(e.contains("Failed to find data source: hive"))
}
}

test("saveAsTable API - format hive") {
val tableName = "tab1"
withTable(tableName) {
val e = intercept[AnalysisException] {
spark.range(10).write.format("hive").mode(SaveMode.Overwrite).saveAsTable(tableName)
}.getMessage
assert(e.contains("Cannot create hive serde table with saveAsTable API"))
}
}

test("create a data source table using hive") {
val tableName = "tab1"
withTable (tableName) {
val e = intercept[AnalysisException] {
sql(
s"""
|CREATE TABLE $tableName
|(col1 int)
|USING hive
""".stripMargin)
}.getMessage
assert(e.contains("Cannot create hive serde table with CREATE TABLE USING"))
}
}

test("create a temp view using hive") {
val tableName = "tab1"
withTable (tableName) {
val e = intercept[ClassNotFoundException] {
sql(
s"""
|CREATE TEMPORARY VIEW $tableName
|(col1 int)
|USING hive
""".stripMargin)
}.getMessage
assert(e.contains("Failed to find data source: hive"))
}
}

test("saveAsTable - source and target are the same table") {
val tableName = "tab1"
withTable(tableName) {
Expand Down