Skip to content

Commit 1ab9731

Browse files
xwu0226gatorsmile
xwu0226
authored and
gatorsmile
committed
[SPARK-19539][SQL] Block duplicate temp table during creation
## What changes were proposed in this pull request? Current `CREATE TEMPORARY TABLE ... ` is deprecated and recommend users to use `CREATE TEMPORARY VIEW ...` And it does not support `IF NOT EXISTS `clause. However, if there is an existing temporary view defined, it is possible to unintentionally replace this existing view by issuing `CREATE TEMPORARY TABLE ...` with the same table/view name. This PR is to disallow `CREATE TEMPORARY TABLE ...` with an existing view name. Under the cover, `CREATE TEMPORARY TABLE ...` will be changed to create temporary view, however, passing in a flag `replace=false`, instead of currently `true`. So when creating temporary view under the cover, if there is existing view with the same name, the operation will be blocked. ## How was this patch tested? New unit test case is added and updated some existing test cases to adapt the new behavior Author: Xin Wu <[email protected]> Closes #16878 from xwu0226/block_duplicate_temp_table.
1 parent 6e45b54 commit 1ab9731

File tree

10 files changed

+160
-137
lines changed

10 files changed

+160
-137
lines changed

‎sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,9 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder {
425425

426426
logWarning(s"CREATE TEMPORARY TABLE ... USING ... is deprecated, please use " +
427427
"CREATE TEMPORARY VIEW ... USING ... instead")
428-
CreateTempViewUsing(table, schema, replace = true, global = false, provider, options)
428+
// Unlike CREATE TEMPORARY VIEW USING, CREATE TEMPORARY TABLE USING does not support
429+
// IF NOT EXISTS. Users are not allowed to replace the existing temp table.
430+
CreateTempViewUsing(table, schema, replace = false, global = false, provider, options)
429431
} else {
430432
CreateTable(tableDesc, mode, None)
431433
}

‎sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1571,7 +1571,7 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
15711571
}
15721572
}
15731573

1574-
test("specifying database name for a temporary table is not allowed") {
1574+
test("specifying database name for a temporary view is not allowed") {
15751575
withTempPath { dir =>
15761576
val path = dir.toURI.toString
15771577
val df =
@@ -1585,23 +1585,23 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
15851585
intercept[AnalysisException] {
15861586
spark.sql(
15871587
s"""
1588-
|CREATE TEMPORARY TABLE db.t
1589-
|USING parquet
1590-
|OPTIONS (
1591-
| path '$path'
1592-
|)
1593-
""".stripMargin)
1588+
|CREATE TEMPORARY VIEW db.t
1589+
|USING parquet
1590+
|OPTIONS (
1591+
| path '$path'
1592+
|)
1593+
""".stripMargin)
15941594
}.getMessage
15951595

15961596
// If you use backticks to quote the name then it's OK.
15971597
spark.sql(
15981598
s"""
1599-
|CREATE TEMPORARY TABLE `db.t`
1599+
|CREATE TEMPORARY VIEW `db.t`
16001600
|USING parquet
16011601
|OPTIONS (
16021602
| path '$path'
16031603
|)
1604-
""".stripMargin)
1604+
""".stripMargin)
16051605
checkAnswer(spark.table("`db.t`"), df)
16061606
}
16071607
}

‎sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala‎

Lines changed: 34 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -903,24 +903,24 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach {
903903
withTempView("show1a", "show2b") {
904904
sql(
905905
"""
906-
|CREATE TEMPORARY TABLE show1a
907-
|USING org.apache.spark.sql.sources.DDLScanSource
908-
|OPTIONS (
909-
| From '1',
910-
| To '10',
911-
| Table 'test1'
912-
|
913-
|)
906+
|CREATE TEMPORARY VIEW show1a
907+
|USING org.apache.spark.sql.sources.DDLScanSource
908+
|OPTIONS (
909+
| From '1',
910+
| To '10',
911+
| Table 'test1'
912+
|
913+
|)
914914
""".stripMargin)
915915
sql(
916916
"""
917-
|CREATE TEMPORARY TABLE show2b
918-
|USING org.apache.spark.sql.sources.DDLScanSource
919-
|OPTIONS (
920-
| From '1',
921-
| To '10',
922-
| Table 'test1'
923-
|)
917+
|CREATE TEMPORARY VIEW show2b
918+
|USING org.apache.spark.sql.sources.DDLScanSource
919+
|OPTIONS (
920+
| From '1',
921+
| To '10',
922+
| Table 'test1'
923+
|)
924924
""".stripMargin)
925925
assert(
926926
sql("SHOW TABLE EXTENDED LIKE 'show*'").count() >= 2)
@@ -958,20 +958,20 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach {
958958
Nil)
959959
}
960960

961-
test("drop table - temporary table") {
961+
test("drop view - temporary view") {
962962
val catalog = spark.sessionState.catalog
963963
sql(
964964
"""
965-
|CREATE TEMPORARY TABLE tab1
966-
|USING org.apache.spark.sql.sources.DDLScanSource
967-
|OPTIONS (
968-
| From '1',
969-
| To '10',
970-
| Table 'test1'
971-
|)
965+
|CREATE TEMPORARY VIEW tab1
966+
|USING org.apache.spark.sql.sources.DDLScanSource
967+
|OPTIONS (
968+
| From '1',
969+
| To '10',
970+
| Table 'test1'
971+
|)
972972
""".stripMargin)
973973
assert(catalog.listTables("default") == Seq(TableIdentifier("tab1")))
974-
sql("DROP TABLE tab1")
974+
sql("DROP VIEW tab1")
975975
assert(catalog.listTables("default") == Nil)
976976
}
977977

@@ -1690,6 +1690,16 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach {
16901690
}
16911691
}
16921692

1693+
test("block creating duplicate temp table") {
1694+
withView("t_temp") {
1695+
sql("CREATE TEMPORARY VIEW t_temp AS SELECT 1, 2")
1696+
val e = intercept[TempTableAlreadyExistsException] {
1697+
sql("CREATE TEMPORARY TABLE t_temp (c3 int, c4 string) USING JSON")
1698+
}.getMessage
1699+
assert(e.contains("Temporary table 't_temp' already exists"))
1700+
}
1701+
}
1702+
16931703
test("truncate table - external table, temporary table, view (not allowed)") {
16941704
import testImplicits._
16951705
withTempPath { tempDir =>

‎sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/RowDataSourceStrategySuite.scala‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,10 @@ class RowDataSourceStrategySuite extends SparkFunSuite with BeforeAndAfter with
5252
conn.commit()
5353
sql(
5454
s"""
55-
|CREATE TEMPORARY TABLE inttypes
55+
|CREATE OR REPLACE TEMPORARY VIEW inttypes
5656
|USING org.apache.spark.sql.jdbc
5757
|OPTIONS (url '$url', dbtable 'TEST.INTTYPES', user 'testUser', password 'testPass')
58-
""".stripMargin.replaceAll("\n", " "))
58+
""".stripMargin.replaceAll("\n", " "))
5959
}
6060

6161
after {

‎sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/csv/CSVSuite.scala‎

Lines changed: 58 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -186,16 +186,17 @@ class CSVSuite extends QueryTest with SharedSQLContext with SQLTestUtils {
186186
}
187187

188188
test("test different encoding") {
189-
// scalastyle:off
190-
spark.sql(
191-
s"""
192-
|CREATE TEMPORARY TABLE carsTable USING csv
193-
|OPTIONS (path "${testFile(carsFile8859)}", header "true",
194-
|charset "iso-8859-1", delimiter "þ")
195-
""".stripMargin.replaceAll("\n", " "))
196-
// scalastyle:on
197-
198-
verifyCars(spark.table("carsTable"), withHeader = true)
189+
withView("carsTable") {
190+
// scalastyle:off
191+
spark.sql(
192+
s"""
193+
|CREATE TEMPORARY VIEW carsTable USING csv
194+
|OPTIONS (path "${testFile(carsFile8859)}", header "true",
195+
|charset "iso-8859-1", delimiter "þ")
196+
""".stripMargin.replaceAll("\n", " "))
197+
// scalastyle:on
198+
verifyCars(spark.table("carsTable"), withHeader = true)
199+
}
199200
}
200201

201202
test("test aliases sep and encoding for delimiter and charset") {
@@ -213,27 +214,31 @@ class CSVSuite extends QueryTest with SharedSQLContext with SQLTestUtils {
213214
}
214215

215216
test("DDL test with tab separated file") {
216-
spark.sql(
217-
s"""
218-
|CREATE TEMPORARY TABLE carsTable USING csv
219-
|OPTIONS (path "${testFile(carsTsvFile)}", header "true", delimiter "\t")
220-
""".stripMargin.replaceAll("\n", " "))
221-
222-
verifyCars(spark.table("carsTable"), numFields = 6, withHeader = true, checkHeader = false)
217+
withView("carsTable") {
218+
spark.sql(
219+
s"""
220+
|CREATE TEMPORARY VIEW carsTable USING csv
221+
|OPTIONS (path "${testFile(carsTsvFile)}", header "true", delimiter "\t")
222+
""".stripMargin.replaceAll("\n", " "))
223+
224+
verifyCars(spark.table("carsTable"), numFields = 6, withHeader = true, checkHeader = false)
225+
}
223226
}
224227

225228
test("DDL test parsing decimal type") {
226-
spark.sql(
227-
s"""
228-
|CREATE TEMPORARY TABLE carsTable
229-
|(yearMade double, makeName string, modelName string, priceTag decimal,
230-
| comments string, grp string)
231-
|USING csv
232-
|OPTIONS (path "${testFile(carsTsvFile)}", header "true", delimiter "\t")
233-
""".stripMargin.replaceAll("\n", " "))
234-
235-
assert(
236-
spark.sql("SELECT makeName FROM carsTable where priceTag > 60000").collect().size === 1)
229+
withView("carsTable") {
230+
spark.sql(
231+
s"""
232+
|CREATE TEMPORARY VIEW carsTable
233+
|(yearMade double, makeName string, modelName string, priceTag decimal,
234+
| comments string, grp string)
235+
|USING csv
236+
|OPTIONS (path "${testFile(carsTsvFile)}", header "true", delimiter "\t")
237+
""".stripMargin.replaceAll("\n", " "))
238+
239+
assert(
240+
spark.sql("SELECT makeName FROM carsTable where priceTag > 60000").collect().size === 1)
241+
}
237242
}
238243

239244
test("test for DROPMALFORMED parsing mode") {
@@ -300,28 +305,34 @@ class CSVSuite extends QueryTest with SharedSQLContext with SQLTestUtils {
300305
}
301306

302307
test("DDL test with empty file") {
303-
spark.sql(s"""
304-
|CREATE TEMPORARY TABLE carsTable
305-
|(yearMade double, makeName string, modelName string, comments string, grp string)
306-
|USING csv
307-
|OPTIONS (path "${testFile(emptyFile)}", header "false")
308-
""".stripMargin.replaceAll("\n", " "))
309-
310-
assert(spark.sql("SELECT count(*) FROM carsTable").collect().head(0) === 0)
308+
withView("carsTable") {
309+
spark.sql(
310+
s"""
311+
|CREATE TEMPORARY VIEW carsTable
312+
|(yearMade double, makeName string, modelName string, comments string, grp string)
313+
|USING csv
314+
|OPTIONS (path "${testFile(emptyFile)}", header "false")
315+
""".stripMargin.replaceAll("\n", " "))
316+
317+
assert(spark.sql("SELECT count(*) FROM carsTable").collect().head(0) === 0)
318+
}
311319
}
312320

313321
test("DDL test with schema") {
314-
spark.sql(s"""
315-
|CREATE TEMPORARY TABLE carsTable
316-
|(yearMade double, makeName string, modelName string, comments string, blank string)
317-
|USING csv
318-
|OPTIONS (path "${testFile(carsFile)}", header "true")
319-
""".stripMargin.replaceAll("\n", " "))
320-
321-
val cars = spark.table("carsTable")
322-
verifyCars(cars, withHeader = true, checkHeader = false, checkValues = false)
323-
assert(
324-
cars.schema.fieldNames === Array("yearMade", "makeName", "modelName", "comments", "blank"))
322+
withView("carsTable") {
323+
spark.sql(
324+
s"""
325+
|CREATE TEMPORARY VIEW carsTable
326+
|(yearMade double, makeName string, modelName string, comments string, blank string)
327+
|USING csv
328+
|OPTIONS (path "${testFile(carsFile)}", header "true")
329+
""".stripMargin.replaceAll("\n", " "))
330+
331+
val cars = spark.table("carsTable")
332+
verifyCars(cars, withHeader = true, checkHeader = false, checkValues = false)
333+
assert(
334+
cars.schema.fieldNames === Array("yearMade", "makeName", "modelName", "comments", "blank"))
335+
}
325336
}
326337

327338
test("save csv") {

0 commit comments

Comments
 (0)