Skip to content

Commit 64fe192

Browse files
imback82cloud-fan
authored andcommitted
[SPARK-30282][SQL] Migrate SHOW TBLPROPERTIES to new framework
### What changes were proposed in this pull request? Use the new framework to resolve the SHOW TBLPROPERTIES command. This PR along with #27243 should update all the existing V2 commands with `UnresolvedV2Relation`. ### Why are the changes needed? This is a part of effort to make the relation lookup behavior consistent: [SPARK-2990](https://issues.apache.org/jira/browse/SPARK-29900). ### Does this PR introduce any user-facing change? Yes `SHOW TBLPROPERTIES temp_view` now fails with `AnalysisException` will be thrown with a message `temp_view is a temp view not table`. Previously, it was returning empty row. ### How was this patch tested? Existing tests Closes #26921 from imback82/consistnet_v2command. Authored-by: Terry Kim <yuminkim@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
1 parent 1881caa commit 64fe192

File tree

11 files changed

+29
-49
lines changed

11 files changed

+29
-49
lines changed

docs/sql-migration-guide.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,8 @@ license: |
338338

339339
- Since Spark 3.0, `ADD FILE` can be used to add file directories as well. Earlier only single files can be added using this command. To restore the behaviour of earlier versions, set `spark.sql.legacy.addDirectory.recursive` to false.
340340

341+
- Since Spark 3.0, `SHOW TBLPROPERTIES` on a temporary view will cause `AnalysisException`. In Spark version 2.4 and earlier, it returned an empty result.
342+
341343
## Upgrading from Spark SQL 2.4 to 2.4.1
342344

343345
- The value of `spark.executor.heartbeatInterval`, when specified without units like "30" rather than "30s", was

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -820,11 +820,6 @@ class Analyzer(
820820
.map(rel => alter.copy(table = rel))
821821
.getOrElse(alter)
822822

823-
case show @ ShowTableProperties(u: UnresolvedV2Relation, _) =>
824-
CatalogV2Util.loadRelation(u.catalog, u.tableName)
825-
.map(rel => show.copy(table = rel))
826-
.getOrElse(show)
827-
828823
case u: UnresolvedV2Relation =>
829824
CatalogV2Util.loadRelation(u.catalog, u.tableName).getOrElse(u)
830825
}

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveCatalogs.scala

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -191,11 +191,6 @@ class ResolveCatalogs(val catalogManager: CatalogManager)
191191

192192
case ShowCurrentNamespaceStatement() =>
193193
ShowCurrentNamespace(catalogManager)
194-
195-
case ShowTablePropertiesStatement(
196-
nameParts @ NonSessionCatalogAndTable(catalog, tbl), propertyKey) =>
197-
val r = UnresolvedV2Relation(nameParts, catalog.asTableCatalog, tbl.asIdentifier)
198-
ShowTableProperties(r, propertyKey)
199194
}
200195

201196
object NonSessionCatalogAndTable {

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3503,8 +3503,8 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
35033503
*/
35043504
override def visitShowTblProperties(
35053505
ctx: ShowTblPropertiesContext): LogicalPlan = withOrigin(ctx) {
3506-
ShowTablePropertiesStatement(
3507-
visitMultipartIdentifier(ctx.table),
3506+
ShowTableProperties(
3507+
UnresolvedTable(visitMultipartIdentifier(ctx.table)),
35083508
Option(ctx.key).map(visitTablePropertyKey))
35093509
}
35103510

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statements.scala

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -438,13 +438,6 @@ case class ShowColumnsStatement(
438438
*/
439439
case class ShowCurrentNamespaceStatement() extends ParsedStatement
440440

441-
/**
442-
* A SHOW TBLPROPERTIES statement, as parsed from SQL
443-
*/
444-
case class ShowTablePropertiesStatement(
445-
tableName: Seq[String],
446-
propertyKey: Option[String]) extends ParsedStatement
447-
448441
/**
449442
* A DESCRIBE FUNCTION statement, as parsed from SQL
450443
*/

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -474,8 +474,10 @@ case class ShowCurrentNamespace(catalogManager: CatalogManager) extends Command
474474
* The logical plan of the SHOW TBLPROPERTIES command that works for v2 catalogs.
475475
*/
476476
case class ShowTableProperties(
477-
table: NamedRelation,
478-
propertyKey: Option[String]) extends Command{
477+
table: LogicalPlan,
478+
propertyKey: Option[String]) extends Command {
479+
override def children: Seq[LogicalPlan] = table :: Nil
480+
479481
override val output: Seq[Attribute] = Seq(
480482
AttributeReference("key", StringType, nullable = false)(),
481483
AttributeReference("value", StringType, nullable = false)())

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DDLParserSuite.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1912,11 +1912,11 @@ class DDLParserSuite extends AnalysisTest {
19121912
test("SHOW TBLPROPERTIES table") {
19131913
comparePlans(
19141914
parsePlan("SHOW TBLPROPERTIES a.b.c"),
1915-
ShowTablePropertiesStatement(Seq("a", "b", "c"), None))
1915+
ShowTableProperties(UnresolvedTable(Seq("a", "b", "c")), None))
19161916

19171917
comparePlans(
19181918
parsePlan("SHOW TBLPROPERTIES a.b.c('propKey1')"),
1919-
ShowTablePropertiesStatement(Seq("a", "b", "c"), Some("propKey1")))
1919+
ShowTableProperties(UnresolvedTable(Seq("a", "b", "c")), Some("propKey1")))
19201920
}
19211921

19221922
test("DESCRIBE FUNCTION") {

sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -485,10 +485,8 @@ class ResolveSessionCatalog(
485485
replace,
486486
viewType)
487487

488-
case ShowTablePropertiesStatement(SessionCatalogAndTable(_, tbl), propertyKey) =>
489-
ShowTablePropertiesCommand(
490-
tbl.asTableIdentifier,
491-
propertyKey)
488+
case ShowTableProperties(r: ResolvedTable, propertyKey) if isSessionCatalog(r.catalog) =>
489+
ShowTablePropertiesCommand(r.identifier.asTableIdentifier, propertyKey)
492490

493491
case DescribeFunctionStatement(CatalogAndIdentifier(catalog, ident), extended) =>
494492
val functionIdent =

sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -900,22 +900,15 @@ case class ShowTablePropertiesCommand(table: TableIdentifier, propertyKey: Optio
900900
}
901901

902902
override def run(sparkSession: SparkSession): Seq[Row] = {
903-
val catalog = sparkSession.sessionState.catalog
904-
905-
if (catalog.isTemporaryTable(table)) {
906-
Seq.empty[Row]
907-
} else {
908-
val catalogTable = sparkSession.sessionState.catalog.getTableMetadata(table)
909-
910-
propertyKey match {
911-
case Some(p) =>
912-
val propValue = catalogTable
913-
.properties
914-
.getOrElse(p, s"Table ${catalogTable.qualifiedName} does not have property: $p")
915-
Seq(Row(propValue))
916-
case None =>
917-
catalogTable.properties.map(p => Row(p._1, p._2)).toSeq
918-
}
903+
val catalogTable = sparkSession.sessionState.catalog.getTableMetadata(table)
904+
propertyKey match {
905+
case Some(p) =>
906+
val propValue = catalogTable
907+
.properties
908+
.getOrElse(p, s"Table ${catalogTable.qualifiedName} does not have property: $p")
909+
Seq(Row(propValue))
910+
case None =>
911+
catalogTable.properties.map(p => Row(p._1, p._2)).toSeq
919912
}
920913
}
921914
}

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
278278
case r: ShowCurrentNamespace =>
279279
ShowCurrentNamespaceExec(r.output, r.catalogManager) :: Nil
280280

281-
case r @ ShowTableProperties(DataSourceV2Relation(table, _, _), propertyKey) =>
281+
case r @ ShowTableProperties(ResolvedTable(_, _, table), propertyKey) =>
282282
ShowTablePropertiesExec(r.output, table, propertyKey) :: Nil
283283

284284
case AlterNamespaceSetOwner(ResolvedNamespace(catalog, namespace), name, typ) =>

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveCommandSuite.scala

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -130,10 +130,10 @@ class HiveCommandSuite extends QueryTest with SQLTestUtils with TestHiveSingleto
130130
}
131131

132132
test("show tblproperties for datasource table - errors") {
133-
val message1 = intercept[NoSuchTableException] {
133+
val message = intercept[AnalysisException] {
134134
sql("SHOW TBLPROPERTIES badtable")
135135
}.getMessage
136-
assert(message1.contains("Table or view 'badtable' not found in database 'default'"))
136+
assert(message.contains("Table not found: badtable"))
137137

138138
// When key is not found, a row containing the error is returned.
139139
checkAnswer(
@@ -147,16 +147,18 @@ class HiveCommandSuite extends QueryTest with SQLTestUtils with TestHiveSingleto
147147
checkAnswer(sql("SHOW TBLPROPERTIES parquet_tab2('`prop2Key`')"), Row("prop2Val"))
148148
}
149149

150-
test("show tblproperties for spark temporary table - empty row") {
150+
test("show tblproperties for spark temporary table - AnalysisException is thrown") {
151151
withTempView("parquet_temp") {
152152
sql(
153153
"""
154154
|CREATE TEMPORARY VIEW parquet_temp (c1 INT, c2 STRING)
155155
|USING org.apache.spark.sql.parquet.DefaultSource
156156
""".stripMargin)
157157

158-
// An empty sequence of row is returned for session temporary table.
159-
checkAnswer(sql("SHOW TBLPROPERTIES parquet_temp"), Nil)
158+
val message = intercept[AnalysisException] {
159+
sql("SHOW TBLPROPERTIES parquet_temp")
160+
}.getMessage
161+
assert(message.contains("parquet_temp is a temp view not table"))
160162
}
161163
}
162164

0 commit comments

Comments
 (0)