Skip to content

[SPARK-43203][SQL] Move all Drop Table case to DataSource V2 #41348

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
Hisoka-X wants to merge 14 commits into from
Closed

[SPARK-43203][SQL] Move all Drop Table case to DataSource V2 #41348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
bfd7c2f
[SPARK-43203][SQL] Move all Drop Table case to V2
Hisoka-X May 28, 2023
5d0677d
move IdentifierImpl to internal
Hisoka-X May 31, 2023
6e1628c
fix test
Hisoka-X May 31, 2023
e237366
fix test
Hisoka-X May 31, 2023
719eb22
fix header
Hisoka-X May 31, 2023
9927414
change hashcode
Hisoka-X May 28, 2023
bfed49e
fix review
Hisoka-X Jun 1, 2023
85164da
Merge branch 'master_' into SPARK-43203_drop_table_to_v2
Hisoka-X Jun 2, 2023
c477802
Merge branch 'master' into SPARK-43203_drop_table_to_v2
Hisoka-X Jun 6, 2023
2a77746
move invalidateCachedTable to V2SessionCatalog
Hisoka-X Jun 7, 2023
b9e5488
fix review
Hisoka-X Jun 7, 2023
9ed2051
revert IdentifierImpl
Hisoka-X Jun 8, 2023
93c632d
remove useless code
Hisoka-X Jun 8, 2023
8282f87
remove public
Hisoka-X Jun 9, 2023
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
2 changes: 1 addition & 1 deletion sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ class ResolveSessionCatalog(val catalogManager: CatalogManager)
c
}

case DropTable(ResolvedV1Identifier(ident), ifExists, purge) =>
case DropTable(ResolvedV1Identifier(ident), ifExists, purge) if conf.useV1Command =>
DropTableCommand(ident, ifExists, isView = false, purge = purge)

// v1 DROP TABLE supports temp view.
Expand Down
6 changes: 6 additions & 0 deletions sql/core/src/main/scala/org/apache/spark/sql/execution/CacheManager.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import org.apache.hadoop.fs.{FileSystem, Path}
import org.apache.spark.internal.Logging
import org.apache.spark.internal.config.ConfigEntry
import org.apache.spark.sql.{Dataset, SparkSession}
import org.apache.spark.sql.catalyst.catalog.HiveTableRelation
import org.apache.spark.sql.catalyst.expressions.{Attribute, SubqueryExpression}
import org.apache.spark.sql.catalyst.optimizer.EliminateResolvedHint
import org.apache.spark.sql.catalyst.plans.logical.{IgnoreCachedData, LogicalPlan, ResolvedHint, SubqueryAlias, View}
Expand Down Expand Up @@ -190,6 +191,11 @@ class CacheManager extends Logging with AdaptiveSparkPlanHelper {
isSameName(ident.qualifier :+ ident.name) &&
isSameName(v1Ident.catalog.toSeq ++ v1Ident.database :+ v1Ident.table)

case SubqueryAlias(ident, HiveTableRelation(catalogTable, _, _, _, _)) =>
Copy link
Member Author

@Hisoka-X Hisoka-X May 29, 2023

Choose a reason for hiding this comment

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

Add support for HiveTableRelation in isMatchedTableOrView, so that can remove Cache normally when use hive catalog.

val v1Ident = catalogTable.identifier
isSameName(ident.qualifier :+ ident.name) &&
isSameName(v1Ident.catalog.toSeq ++ v1Ident.database :+ v1Ident.table)

case _ => false
}
}
Expand Down
40 changes: 31 additions & 9 deletions sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2SessionCatalog.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,12 @@ class V2SessionCatalog(catalog: SessionCatalog)
}

override def loadTable(ident: Identifier): Table = {
V1Table(catalog.getTableMetadata(ident.asTableIdentifier))
try {
V1Table(catalog.getTableMetadata(ident.asTableIdentifier))
} catch {
case _: NoSuchDatabaseException =>
Copy link
Member Author

@Hisoka-X Hisoka-X May 29, 2023

Choose a reason for hiding this comment

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

loadTable will return NoSuchDatabaseException, not same as v1 behavior.

Copy link
Contributor

@cloud-fan cloud-fan Jun 1, 2023

Choose a reason for hiding this comment

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

does it cause test failures?

Copy link
Member Author

@Hisoka-X Hisoka-X Jun 1, 2023

Choose a reason for hiding this comment

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

Yes, the DerbyTableCatalogSuite:SPARK-42978: RENAME cannot qualify a new-table-Name with a schema-Name. will failed without this change.

throw QueryCompilationErrors.noSuchTableError(ident)
}
}

override def loadTable(ident: Identifier, timestamp: Long): Table = {
Expand Down Expand Up @@ -110,6 +115,10 @@ class V2SessionCatalog(catalog: SessionCatalog)
createTable(ident, CatalogV2Util.v2ColumnsToStructType(columns), partitions, properties)
}

override def purgeTable(ident: Identifier): Boolean = {
dropTableInternal(ident, purge = true)
}

// TODO: remove it when no tests calling this deprecated method.
override def createTable(
ident: Identifier,
Expand Down Expand Up @@ -194,16 +203,29 @@ class V2SessionCatalog(catalog: SessionCatalog)
}

override def dropTable(ident: Identifier): Boolean = {
dropTableInternal(ident)
}

private def dropTableInternal(ident: Identifier, purge: Boolean = false): Boolean = {
try {
if (loadTable(ident) != null) {
catalog.dropTable(
ident.asTableIdentifier,
ignoreIfNotExists = true,
purge = true /* skip HDFS trash */)
true
} else {
false
loadTable(ident) match {
case V1Table(v1Table) if v1Table.tableType == CatalogTableType.VIEW =>
throw QueryCompilationErrors.wrongCommandForObjectTypeError(
operation = "DROP TABLE",
requiredType = s"${CatalogTableType.EXTERNAL.name} or" +
s" ${CatalogTableType.MANAGED.name}",
objectName = v1Table.qualifiedName,
foundType = v1Table.tableType.name,
alternative = "DROP VIEW"
)
case _ =>
}
catalog.invalidateCachedTable(ident.asTableIdentifier)
catalog.dropTable(
ident.asTableIdentifier,
ignoreIfNotExists = true,
purge = purge)
true
} catch {
case _: NoSuchTableException =>
false
Expand Down
3 changes: 2 additions & 1 deletion sql/core/src/test/resources/sql-tests/analyzer-results/ansi/conditional-functions.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,5 @@ Project [CASE WHEN (1 > 2) THEN (cast(1 as double) / cast(0 as double)) ELSE cas
-- !query
DROP TABLE conditional_t
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`conditional_t`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.conditional_t
3 changes: 2 additions & 1 deletion ...re/src/test/resources/sql-tests/analyzer-results/ansi/decimalArithmeticOperations.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,4 +166,5 @@ Project [(10123456789012345678901234567890.123456 / 10.0) AS (101234567890123456
-- !query
drop table decimals_test
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`decimals_test`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.decimals_test
3 changes: 2 additions & 1 deletion sql/core/src/test/resources/sql-tests/analyzer-results/change-column.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,8 @@ org.apache.spark.sql.AnalysisException
-- !query
DROP TABLE test_change
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`test_change`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.test_change


-- !query
Expand Down
12 changes: 8 additions & 4 deletions sql/core/src/test/resources/sql-tests/analyzer-results/charvarchar.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -659,25 +659,29 @@ Project [cast(c7#x as string) AS c7#x, cast(c8#x as string) AS c8#x, cast(v#x as
-- !query
drop table char_tbl1
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`char_tbl1`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.char_tbl1


-- !query
drop table char_tbl2
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`char_tbl2`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.char_tbl2


-- !query
drop table char_tbl3
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`char_tbl3`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.char_tbl3


-- !query
drop table char_tbl4
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`char_tbl4`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.char_tbl4


-- !query
Expand Down
6 changes: 4 additions & 2 deletions sql/core/src/test/resources/sql-tests/analyzer-results/columnresolution-negative.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,8 @@ CreateViewCommand `spark_catalog`.`mydb1`.`v1`, SELECT * FROM t1, false, false,
-- !query
DROP TABLE t1
-- !query analysis
DropTableCommand `spark_catalog`.`mydb1`.`t1`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), mydb1.t1


-- !query
Expand Down Expand Up @@ -441,7 +442,8 @@ CreateViewCommand `v2`, SELECT * FROM t1, false, false, LocalTempView, true
-- !query
DROP TABLE t1
-- !query analysis
DropTableCommand `spark_catalog`.`mydb2`.`t1`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), mydb2.t1


-- !query
Expand Down
3 changes: 2 additions & 1 deletion sql/core/src/test/resources/sql-tests/analyzer-results/decimalArithmeticOperations.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,4 +442,5 @@ Project [(10123456789012345678901234567890.123456 / 10.0) AS (101234567890123456
-- !query
drop table decimals_test
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`decimals_test`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.decimals_test
3 changes: 2 additions & 1 deletion sql/core/src/test/resources/sql-tests/analyzer-results/describe-part-after-analyze.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,5 @@ DescribeTableCommand `spark_catalog`.`default`.`t`, [ds=2017-09-01, hr=5], true,
-- !query
DROP TABLE t
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`t`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.t
6 changes: 4 additions & 2 deletions sql/core/src/test/resources/sql-tests/analyzer-results/describe-query.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,12 @@ ExplainCommand DescribeQueryCommand WITH s AS (SELECT 'hello' as col1) SELECT *
-- !query
DROP TABLE desc_temp1
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`desc_temp1`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.desc_temp1


-- !query
DROP TABLE desc_temp2
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`desc_temp2`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.desc_temp2
6 changes: 4 additions & 2 deletions ...re/src/test/resources/sql-tests/analyzer-results/describe-table-after-alter-table.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ DescribeTableCommand `spark_catalog`.`default`.`table_with_comment`, true, [col_
-- !query
DROP TABLE table_with_comment
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`table_with_comment`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.table_with_comment


-- !query
Expand Down Expand Up @@ -68,4 +69,5 @@ DescribeTableCommand `spark_catalog`.`default`.`table_comment`, true, [col_name#
-- !query
DROP TABLE table_comment
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`table_comment`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.table_comment
3 changes: 2 additions & 1 deletion sql/core/src/test/resources/sql-tests/analyzer-results/describe.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,8 @@ ExplainCommand 'DescribeRelation [c=Us, d=2], false, [col_name#x, data_type#x, c
-- !query
DROP TABLE t
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`t`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.t


-- !query
Expand Down
18 changes: 12 additions & 6 deletions sql/core/src/test/resources/sql-tests/analyzer-results/explain-aqe.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,31 +202,36 @@ ExplainCommand 'InsertIntoStatement 'UnresolvedRelation [explain_temp5], [], fal
-- !query
DROP TABLE explain_temp1
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`explain_temp1`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.explain_temp1


-- !query
DROP TABLE explain_temp2
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`explain_temp2`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.explain_temp2


-- !query
DROP TABLE explain_temp3
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`explain_temp3`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.explain_temp3


-- !query
DROP TABLE explain_temp4
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`explain_temp4`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.explain_temp4


-- !query
DROP TABLE explain_temp5
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`explain_temp5`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.explain_temp5


-- !query
Expand All @@ -244,4 +249,5 @@ ExplainCommand 'Project [*], SimpleMode
-- !query
DROP TABLE t
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`t`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.t
6 changes: 4 additions & 2 deletions sql/core/src/test/resources/sql-tests/analyzer-results/explain-cbo.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,12 @@ ExplainCommand CTE [max_store_sales, best_ss_customer], CostMode
-- !query
DROP TABLE explain_temp1
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`explain_temp1`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.explain_temp1


-- !query
DROP TABLE explain_temp2
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`explain_temp2`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.explain_temp2
18 changes: 12 additions & 6 deletions sql/core/src/test/resources/sql-tests/analyzer-results/explain.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,31 +202,36 @@ ExplainCommand 'InsertIntoStatement 'UnresolvedRelation [explain_temp5], [], fal
-- !query
DROP TABLE explain_temp1
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`explain_temp1`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.explain_temp1


-- !query
DROP TABLE explain_temp2
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`explain_temp2`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.explain_temp2


-- !query
DROP TABLE explain_temp3
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`explain_temp3`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.explain_temp3


-- !query
DROP TABLE explain_temp4
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`explain_temp4`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.explain_temp4


-- !query
DROP TABLE explain_temp5
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`explain_temp5`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.explain_temp5


-- !query
Expand All @@ -244,4 +249,5 @@ ExplainCommand 'Project [*], SimpleMode
-- !query
DROP TABLE t
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`t`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.t
15 changes: 10 additions & 5 deletions sql/core/src/test/resources/sql-tests/analyzer-results/identifier-clause.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,8 @@ SetNamespaceCommand [default]
-- !query
DROP TABLE s.tab
-- !query analysis
DropTableCommand `spark_catalog`.`s`.`tab`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), s.tab


-- !query
Expand Down Expand Up @@ -210,7 +211,8 @@ CreateDataSourceTableCommand `spark_catalog`.`default`.`tab`, false
-- !query
DROP TABLE IF EXISTS IDENTIFIER('ta' || 'b')
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`tab`, true, false, false
DropTable true, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.tab


-- !query
Expand All @@ -236,7 +238,8 @@ CreateDataSourceTableCommand `spark_catalog`.`identifier_clauses`.`tab`, false
-- !query
DROP TABLE IF EXISTS IDENTIFIER('identifier_clauses.' || 'tab')
-- !query analysis
DropTableCommand `spark_catalog`.`identifier_clauses`.`tab`, true, false, false
DropTable true, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), identifier_clauses.tab


-- !query
Expand Down Expand Up @@ -278,7 +281,8 @@ UncacheTable false, true
-- !query
DROP TABLE IF EXISTS IDENTIFIER('ta' || 'b')
-- !query analysis
DropTableCommand `spark_catalog`.`identifier_clauses`.`tab`, true, false, false
DropTable true, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), identifier_clauses.tab


-- !query
Expand Down Expand Up @@ -383,7 +387,8 @@ TruncateTableCommand `spark_catalog`.`default`.`tab`
-- !query
DROP TABLE IF EXISTS tab
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`tab`, true, false, false
DropTable true, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.tab


-- !query
Expand Down
3 changes: 2 additions & 1 deletion sql/core/src/test/resources/sql-tests/analyzer-results/null-handling.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,4 +236,5 @@ Project [b#x, c#x, equal_null(b#x, c#x) AS equal_null(b, c)#x, equal_null(c#x, b
-- !query
drop table t1
-- !query analysis
DropTableCommand `spark_catalog`.`default`.`t1`, false, false, false
DropTable false, false
+- ResolvedIdentifier V2SessionCatalog(spark_catalog), default.t1
Loading