Skip to content

[SPARK-19539][SQL] Block duplicate temp table during creation #16878

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

[SPARK-19539][SQL] Block duplicate temp table during creation #16878

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0afe3cb
don't allow create temporary table to replace existing temp view
xwu0226 Feb 10, 2017
290fa15
update testcases
xwu0226 Feb 10, 2017
4a3a06a
update testcases
xwu0226 Feb 10, 2017
f7253c5
update testcases
xwu0226 Feb 13, 2017
6683d8c
update upon review
xwu0226 Feb 13, 2017
3a989cf
fix indention
xwu0226 Feb 13, 2017
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
4 changes: 3 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 @@ -425,7 +425,9 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder {

logWarning(s"CREATE TEMPORARY TABLE ... USING ... is deprecated, please use " +
Copy link
Contributor

@cloud-fan cloud-fan Feb 10, 2017

Choose a reason for hiding this comment

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

can you check when did we forbid this? maybe it's time to fully remove the support

Copy link
Contributor Author

@xwu0226 xwu0226 Feb 10, 2017

Choose a reason for hiding this comment

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

@cloud-fan Thanks for reviewing! I found that this deprecation was added by your PR #14482 back on 8/5/2016. Do you think it is the right time to remove CREATE TEMPORARY TABLE... completely?

Copy link
Contributor

@cloud-fan cloud-fan Feb 10, 2017

Choose a reason for hiding this comment

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

no, it should be earlier: https://github.com/apache/spark/pull/14482/files#diff-7253a38df7e111ecf6b1ef71feba383bL425

Copy link
Contributor Author

@xwu0226 xwu0226 Feb 10, 2017

Choose a reason for hiding this comment

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

Are we referring to the same PR? I found the change here https://github.com/apache/spark/pull/14482/files#diff-1bb4f7bd5a2656f48bcd3c857167a11bR362, where this WARN message is added in SparkSQLParser.scala

Copy link
Contributor

@cloud-fan cloud-fan Feb 10, 2017

Choose a reason for hiding this comment

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

I mean it was moved from other places, so this warning should have been added earlier.

Copy link
Contributor Author

@xwu0226 xwu0226 Feb 10, 2017

Choose a reason for hiding this comment

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

I see. sorry. I misunderstood. Let me check further.

Copy link
Contributor Author

@xwu0226 xwu0226 Feb 10, 2017

Choose a reason for hiding this comment

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

#13414 deprecated this. Merged on Jun 7th, 2016.

Copy link
Contributor

@cloud-fan cloud-fan Feb 12, 2017

Choose a reason for hiding this comment

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

it was merged to 2.0, let's keep it

"CREATE TEMPORARY VIEW ... USING ... instead")
CreateTempViewUsing(table, schema, replace = true, global = false, provider, options)
// Unlike CREATE TEMPORARY VIEW USING, CREATE TEMPORARY TABLE USING does not support
// IF NOT EXISTS. Users are not allowed to replace the existing temp table.
CreateTempViewUsing(table, schema, replace = false, global = false, provider, options)
} else {
CreateTable(tableDesc, mode, None)
}
Expand Down
6 changes: 3 additions & 3 deletions sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1571,7 +1571,7 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
}
}

test("specifying database name for a temporary table is not allowed") {
test("specifying database name for a temporary view is not allowed") {
withTempPath { dir =>
val path = dir.toURI.toString
val df =
Expand All @@ -1585,7 +1585,7 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
intercept[AnalysisException] {
spark.sql(
s"""
|CREATE TEMPORARY TABLE db.t
|CREATE TEMPORARY VIEW db.t
|USING parquet
|OPTIONS (
| path '$path'
Expand All @@ -1596,7 +1596,7 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
// If you use backticks to quote the name then it's OK.
spark.sql(
s"""
|CREATE TEMPORARY TABLE `db.t`
|CREATE TEMPORARY VIEW `db.t`
|USING parquet
|OPTIONS (
| path '$path'
Expand Down
20 changes: 15 additions & 5 deletions sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -903,7 +903,7 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach {
withTempView("show1a", "show2b") {
sql(
"""
|CREATE TEMPORARY TABLE show1a
|CREATE TEMPORARY VIEW show1a
|USING org.apache.spark.sql.sources.DDLScanSource
|OPTIONS (
| From '1',
Expand All @@ -914,7 +914,7 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach {
""".stripMargin)
sql(
"""
|CREATE TEMPORARY TABLE show2b
|CREATE TEMPORARY VIEW show2b
|USING org.apache.spark.sql.sources.DDLScanSource
|OPTIONS (
| From '1',
Expand Down Expand Up @@ -958,11 +958,11 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach {
Nil)
}

test("drop table - temporary table") {
test("drop view - temporary view") {
val catalog = spark.sessionState.catalog
sql(
"""
|CREATE TEMPORARY TABLE tab1
|CREATE TEMPORARY VIEW tab1
|USING org.apache.spark.sql.sources.DDLScanSource
|OPTIONS (
| From '1',
Expand All @@ -971,7 +971,7 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach {
|)
""".stripMargin)
assert(catalog.listTables("default") == Seq(TableIdentifier("tab1")))
sql("DROP TABLE tab1")
sql("DROP view tab1")
Copy link
Contributor

@cloud-fan cloud-fan Feb 13, 2017

Choose a reason for hiding this comment

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

nit: upper case "view"

assert(catalog.listTables("default") == Nil)
}

Expand Down Expand Up @@ -1690,6 +1690,16 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach {
}
}

test("block creating duplicate temp table") {
withView("t_temp") {
sql("CREATE TEMPORARY VIEW t_temp AS SELECT 1, 2")
val e = intercept[TempTableAlreadyExistsException] {
sql("CREATE TEMPORARY TABLE t_temp (c3 int, c4 string) USING JSON")
}.getMessage
assert(e.contains("Temporary table 't_temp' already exists"))
}
}

test("truncate table - external table, temporary table, view (not allowed)") {
import testImplicits._
withTempPath { tempDir =>
Expand Down
2 changes: 1 addition & 1 deletion ...rc/test/scala/org/apache/spark/sql/execution/datasources/RowDataSourceStrategySuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class RowDataSourceStrategySuite extends SparkFunSuite with BeforeAndAfter with
conn.commit()
sql(
s"""
|CREATE TEMPORARY TABLE inttypes
|CREATE OR REPLACE TEMPORARY VIEW inttypes
|USING org.apache.spark.sql.jdbc
|OPTIONS (url '$url', dbtable 'TEST.INTTYPES', user 'testUser', password 'testPass')
""".stripMargin.replaceAll("\n", " "))
Expand Down
56 changes: 34 additions & 22 deletions sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/csv/CSVSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,15 +187,17 @@ class CSVSuite extends QueryTest with SharedSQLContext with SQLTestUtils {

test("test different encoding") {
// scalastyle:off
spark.sql(
s"""
|CREATE TEMPORARY TABLE carsTable USING csv
withView("carsTable") {
spark.sql(
s"""
|CREATE TEMPORARY VIEW carsTable USING csv
Copy link
Contributor

@cloud-fan cloud-fan Feb 13, 2017

Choose a reason for hiding this comment

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

the indention is wrong

|OPTIONS (path "${testFile(carsFile8859)}", header "true",
|charset "iso-8859-1", delimiter "þ")
""".stripMargin.replaceAll("\n", " "))
// scalastyle:on

verifyCars(spark.table("carsTable"), withHeader = true)
verifyCars(spark.table("carsTable"), withHeader = true)
}
}

test("test aliases sep and encoding for delimiter and charset") {
Expand All @@ -213,27 +215,31 @@ class CSVSuite extends QueryTest with SharedSQLContext with SQLTestUtils {
}

test("DDL test with tab separated file") {
spark.sql(
s"""
|CREATE TEMPORARY TABLE carsTable USING csv
withView("carsTable") {
spark.sql(
s"""
|CREATE TEMPORARY VIEW carsTable USING csv
Copy link
Contributor

@cloud-fan cloud-fan Feb 13, 2017

Choose a reason for hiding this comment

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

the indention is wrong

|OPTIONS (path "${testFile(carsTsvFile)}", header "true", delimiter "\t")
""".stripMargin.replaceAll("\n", " "))

verifyCars(spark.table("carsTable"), numFields = 6, withHeader = true, checkHeader = false)
verifyCars(spark.table("carsTable"), numFields = 6, withHeader = true, checkHeader = false)
}
}

test("DDL test parsing decimal type") {
spark.sql(
s"""
|CREATE TEMPORARY TABLE carsTable
withView("carsTable") {
spark.sql(
s"""
|CREATE TEMPORARY VIEW carsTable
Copy link
Contributor

@cloud-fan cloud-fan Feb 13, 2017

Choose a reason for hiding this comment

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

the indention is wrong

|(yearMade double, makeName string, modelName string, priceTag decimal,
| comments string, grp string)
|USING csv
|OPTIONS (path "${testFile(carsTsvFile)}", header "true", delimiter "\t")
""".stripMargin.replaceAll("\n", " "))

assert(
spark.sql("SELECT makeName FROM carsTable where priceTag > 60000").collect().size === 1)
assert(
spark.sql("SELECT makeName FROM carsTable where priceTag > 60000").collect().size === 1)
}
}

test("test for DROPMALFORMED parsing mode") {
Expand Down Expand Up @@ -300,28 +306,34 @@ class CSVSuite extends QueryTest with SharedSQLContext with SQLTestUtils {
}

test("DDL test with empty file") {
spark.sql(s"""
|CREATE TEMPORARY TABLE carsTable
withView("carsTable") {
spark.sql(
s"""
|CREATE TEMPORARY VIEW carsTable
Copy link
Contributor

@cloud-fan cloud-fan Feb 13, 2017

Choose a reason for hiding this comment

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

the indention is wrong

|(yearMade double, makeName string, modelName string, comments string, grp string)
|USING csv
|OPTIONS (path "${testFile(emptyFile)}", header "false")
""".stripMargin.replaceAll("\n", " "))

assert(spark.sql("SELECT count(*) FROM carsTable").collect().head(0) === 0)
assert(spark.sql("SELECT count(*) FROM carsTable").collect().head(0) === 0)
}
}

test("DDL test with schema") {
spark.sql(s"""
|CREATE TEMPORARY TABLE carsTable
withView("carsTable") {
spark.sql(
s"""
|CREATE TEMPORARY VIEW carsTable
Copy link
Contributor

@cloud-fan cloud-fan Feb 13, 2017

Choose a reason for hiding this comment

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

the indention is wrong

|(yearMade double, makeName string, modelName string, comments string, blank string)
|USING csv
|OPTIONS (path "${testFile(carsFile)}", header "true")
""".stripMargin.replaceAll("\n", " "))

val cars = spark.table("carsTable")
verifyCars(cars, withHeader = true, checkHeader = false, checkValues = false)
assert(
cars.schema.fieldNames === Array("yearMade", "makeName", "modelName", "comments", "blank"))
val cars = spark.table("carsTable")
verifyCars(cars, withHeader = true, checkHeader = false, checkValues = false)
assert(
cars.schema.fieldNames === Array("yearMade", "makeName", "modelName", "comments", "blank"))
}
}

test("save csv") {
Expand Down
26 changes: 13 additions & 13 deletions sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,22 +75,22 @@ class JDBCSuite extends SparkFunSuite

sql(
s"""
|CREATE TEMPORARY TABLE foobar
|CREATE OR REPLACE TEMPORARY VIEW foobar
|USING org.apache.spark.sql.jdbc
|OPTIONS (url '$url', dbtable 'TEST.PEOPLE', user 'testUser', password 'testPass')
""".stripMargin.replaceAll("\n", " "))

sql(
s"""
|CREATE TEMPORARY TABLE fetchtwo
|CREATE OR REPLACE TEMPORARY VIEW fetchtwo
|USING org.apache.spark.sql.jdbc
|OPTIONS (url '$url', dbtable 'TEST.PEOPLE', user 'testUser', password 'testPass',
| ${JDBCOptions.JDBC_BATCH_FETCH_SIZE} '2')
""".stripMargin.replaceAll("\n", " "))

sql(
s"""
|CREATE TEMPORARY TABLE parts
|CREATE OR REPLACE TEMPORARY VIEW parts
|USING org.apache.spark.sql.jdbc
|OPTIONS (url '$url', dbtable 'TEST.PEOPLE', user 'testUser', password 'testPass',
| partitionColumn 'THEID', lowerBound '1', upperBound '4', numPartitions '3')
Expand All @@ -105,7 +105,7 @@ class JDBCSuite extends SparkFunSuite
conn.commit()
sql(
s"""
|CREATE TEMPORARY TABLE inttypes
|CREATE OR REPLACE TEMPORARY VIEW inttypes
|USING org.apache.spark.sql.jdbc
|OPTIONS (url '$url', dbtable 'TEST.INTTYPES', user 'testUser', password 'testPass')
""".stripMargin.replaceAll("\n", " "))
Expand All @@ -122,7 +122,7 @@ class JDBCSuite extends SparkFunSuite
stmt.executeUpdate()
sql(
s"""
|CREATE TEMPORARY TABLE strtypes
|CREATE OR REPLACE TEMPORARY VIEW strtypes
|USING org.apache.spark.sql.jdbc
|OPTIONS (url '$url', dbtable 'TEST.STRTYPES', user 'testUser', password 'testPass')
""".stripMargin.replaceAll("\n", " "))
Expand All @@ -136,7 +136,7 @@ class JDBCSuite extends SparkFunSuite
conn.commit()
sql(
s"""
|CREATE TEMPORARY TABLE timetypes
|CREATE OR REPLACE TEMPORARY VIEW timetypes
|USING org.apache.spark.sql.jdbc
|OPTIONS (url '$url', dbtable 'TEST.TIMETYPES', user 'testUser', password 'testPass')
""".stripMargin.replaceAll("\n", " "))
Expand All @@ -151,7 +151,7 @@ class JDBCSuite extends SparkFunSuite
conn.commit()
sql(
s"""
|CREATE TEMPORARY TABLE flttypes
|CREATE OR REPLACE TEMPORARY VIEW flttypes
|USING org.apache.spark.sql.jdbc
|OPTIONS (url '$url', dbtable 'TEST.FLTTYPES', user 'testUser', password 'testPass')
""".stripMargin.replaceAll("\n", " "))
Expand All @@ -168,7 +168,7 @@ class JDBCSuite extends SparkFunSuite
conn.commit()
sql(
s"""
|CREATE TEMPORARY TABLE nulltypes
|CREATE OR REPLACE TEMPORARY VIEW nulltypes
|USING org.apache.spark.sql.jdbc
|OPTIONS (url '$url', dbtable 'TEST.NULLTYPES', user 'testUser', password 'testPass')
""".stripMargin.replaceAll("\n", " "))
Expand Down Expand Up @@ -198,7 +198,7 @@ class JDBCSuite extends SparkFunSuite

sql(
s"""
|CREATE TEMPORARY TABLE nullparts
|CREATE OR REPLACE TEMPORARY VIEW nullparts
|USING org.apache.spark.sql.jdbc
|OPTIONS (url '$url', dbtable 'TEST.EMP', user 'testUser', password 'testPass',
|partitionColumn '"Dept"', lowerBound '1', upperBound '4', numPartitions '3')
Expand All @@ -214,7 +214,7 @@ class JDBCSuite extends SparkFunSuite

sql(
s"""
|CREATE TEMPORARY TABLE mixedCaseCols
|CREATE OR REPLACE TEMPORARY VIEW mixedCaseCols
|USING org.apache.spark.sql.jdbc
|OPTIONS (url '$url', dbtable 'TEST."mixedCaseCols"', user 'testUser', password 'testPass')
""".stripMargin.replaceAll("\n", " "))
Expand Down Expand Up @@ -371,7 +371,7 @@ class JDBCSuite extends SparkFunSuite
// Regression test for bug SPARK-7345
sql(
s"""
|CREATE TEMPORARY TABLE renamed
|CREATE OR REPLACE TEMPORARY VIEW renamed
|USING org.apache.spark.sql.jdbc
|OPTIONS (url '$url', dbtable '(select NAME as NAME1, NAME as NAME2 from TEST.PEOPLE)',
|user 'testUser', password 'testPass')
Expand Down Expand Up @@ -589,7 +589,7 @@ class JDBCSuite extends SparkFunSuite
test("SQL query as table name") {
sql(
s"""
|CREATE TEMPORARY TABLE hack
|CREATE OR REPLACE TEMPORARY VIEW hack
|USING org.apache.spark.sql.jdbc
|OPTIONS (url '$url', dbtable '(SELECT B, B*B FROM TEST.FLTTYPES)',
| user 'testUser', password 'testPass')
Expand All @@ -606,7 +606,7 @@ class JDBCSuite extends SparkFunSuite
intercept[JdbcSQLException] {
sql(
s"""
|CREATE TEMPORARY TABLE abc
|CREATE OR REPLACE TEMPORARY VIEW abc
|USING org.apache.spark.sql.jdbc
|OPTIONS (url '$url', dbtable '(SELECT _ROWID_ FROM test.people)',
| user 'testUser', password 'testPass')
Expand Down
2 changes: 1 addition & 1 deletion 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 @@ -83,7 +83,7 @@ class DDLTestSuite extends DataSourceTest with SharedSQLContext {
super.beforeAll()
sql(
"""
|CREATE TEMPORARY TABLE ddlPeople
|CREATE OR REPLACE TEMPORARY VIEW ddlPeople
|USING org.apache.spark.sql.sources.DDLScanSource
|OPTIONS (
| From '1',
Expand Down
4 changes: 2 additions & 2 deletions sql/core/src/test/scala/org/apache/spark/sql/sources/InsertSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class InsertSuite extends DataSourceTest with SharedSQLContext {
spark.read.json(rdd).createOrReplaceTempView("jt")
sql(
s"""
|CREATE TEMPORARY TABLE jsonTable (a int, b string)
|CREATE TEMPORARY VIEW jsonTable (a int, b string)
|USING org.apache.spark.sql.json.DefaultSource
|OPTIONS (
| path '${path.toURI.toString}'
Expand Down Expand Up @@ -293,7 +293,7 @@ class InsertSuite extends DataSourceTest with SharedSQLContext {
test("it's not allowed to insert into a relation that is not an InsertableRelation") {
sql(
"""
|CREATE TEMPORARY TABLE oneToTen
|CREATE TEMPORARY VIEW oneToTen
|USING org.apache.spark.sql.sources.SimpleScanSource
|OPTIONS (
| From '1',
Expand Down
4 changes: 2 additions & 2 deletions sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveCommandSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ class HiveCommandSuite extends QueryTest with SQLTestUtils with TestHiveSingleto
withTempView("parquet_temp") {
sql(
"""
|CREATE TEMPORARY TABLE parquet_temp (c1 INT, c2 STRING)
|CREATE TEMPORARY VIEW parquet_temp (c1 INT, c2 STRING)
|USING org.apache.spark.sql.parquet.DefaultSource
""".stripMargin)

Expand Down Expand Up @@ -401,7 +401,7 @@ class HiveCommandSuite extends QueryTest with SQLTestUtils with TestHiveSingleto
withTempView("parquet_temp") {
sql(
"""
|CREATE TEMPORARY TABLE parquet_temp (c1 INT, c2 STRING)
|CREATE TEMPORARY VIEW parquet_temp (c1 INT, c2 STRING)
|USING org.apache.spark.sql.parquet.DefaultSource
""".stripMargin)
// An empty sequence of row is returned for session temporary table.
Expand Down
6 changes: 3 additions & 3 deletions sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1290,7 +1290,7 @@ class SQLQuerySuite extends QueryTest with SQLTestUtils with TestHiveSingleton {
"interval 4 minutes 59 seconds 889 milliseconds 987 microseconds")))
}

test("specifying database name for a temporary table is not allowed") {
test("specifying database name for a temporary view is not allowed") {
withTempPath { dir =>
val path = dir.toURI.toString
val df = sparkContext.parallelize(1 to 10).map(i => (i, i.toString)).toDF("num", "str")
Expand All @@ -1303,7 +1303,7 @@ class SQLQuerySuite extends QueryTest with SQLTestUtils with TestHiveSingleton {
intercept[AnalysisException] {
spark.sql(
s"""
|CREATE TEMPORARY TABLE db.t
|CREATE TEMPORARY VIEW db.t
|USING parquet
|OPTIONS (
| path '$path'
Expand All @@ -1314,7 +1314,7 @@ class SQLQuerySuite extends QueryTest with SQLTestUtils with TestHiveSingleton {
// If you use backticks to quote the name then it's OK.
spark.sql(
s"""
|CREATE TEMPORARY TABLE `db.t`
|CREATE TEMPORARY VIEW `db.t`
|USING parquet
|OPTIONS (
| path '$path'
Expand Down