Skip to content

Commit

Permalink
[SPARK-30885][SQL] V1 table name should be fully qualified if catalog…
Browse files Browse the repository at this point in the history
… name is provided

### What changes were proposed in this pull request?

For the following:
```
CREATE TABLE t USING json AS SELECT 1 AS i
SELECT * FROM spark_catalog.t
```
`spark_catalog.t` is resolved to `spark_catalog.default.t` assuming the current namespace is `default`. However, this is not consistent with V2 behavior where the namespace must be specified if the catalog name is provided. This PR proposes to fix this inconsistency.

### Why are the changes needed?

To be consistent with V2 table naming scheme in SQL commands.

### Does this PR introduce any user-facing change?

Yes, now the user has to specify the namespace if the catalog name is provided. For example,
```
SELECT * FROM spark_catalog.t # Will throw AnalysisException with 'Session catalog cannot have an empty namespace: spark_catalog.t'
SELECT * FROM spark_catalog.default.t # OK
```

### How was this patch tested?

Added new tests

Closes apache#27642 from imback82/disallow_spark_catalog_wihtout_db.

Authored-by: Terry Kim <yuminkim@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
  • Loading branch information
imback82 authored and Seongjin Cho committed Apr 14, 2020
1 parent c2ce566 commit 9b86f36
Show file tree
Hide file tree
Showing 18 changed files with 289 additions and 236 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -250,20 +250,9 @@ class ResolveSessionCatalog(
case DescribeRelation(ResolvedView(ident), partitionSpec, isExtended) =>
DescribeTableCommand(ident.asTableIdentifier, partitionSpec, isExtended)

case DescribeColumnStatement(
SessionCatalogAndTable(catalog, tbl), colNameParts, isExtended) =>
loadTable(catalog, tbl.asIdentifier).collect {
// `V1Table` also includes permanent views.
case v1Table: V1Table =>
DescribeColumnCommand(tbl.asTableIdentifier, colNameParts, isExtended)
}.getOrElse {
if (isTempView(tbl)) {
// v1 DESCRIBE COLUMN supports temp view.
DescribeColumnCommand(tbl.asTableIdentifier, colNameParts, isExtended)
} else {
throw new AnalysisException("Describing columns is not supported for v2 tables.")
}
}
case DescribeColumnStatement(tbl, colNameParts, isExtended) =>
val name = parseTempViewOrV1Table(tbl, "Describing columns")
DescribeColumnCommand(name.asTableIdentifier, colNameParts, isExtended)

// For CREATE TABLE [AS SELECT], we should use the v1 command if the catalog is resolved to the
// session catalog and the table provider is not v2.
Expand Down Expand Up @@ -396,7 +385,7 @@ class ResolveSessionCatalog(
}

case AnalyzeColumnStatement(tbl, columnNames, allColumns) =>
val v1TableName = parseV1Table(tbl, "ANALYZE TABLE")
val v1TableName = parseTempViewOrV1Table(tbl, "ANALYZE TABLE")
AnalyzeColumnCommand(v1TableName.asTableIdentifier, columnNames, allColumns)

case RepairTableStatement(tbl) =>
Expand All @@ -415,8 +404,8 @@ class ResolveSessionCatalog(
partition)

case ShowCreateTableStatement(tbl, asSerde) if !asSerde =>
val v1TableName = parseV1Table(tbl, "SHOW CREATE TABLE")
ShowCreateTableCommand(v1TableName.asTableIdentifier)
val name = parseTempViewOrV1Table(tbl, "SHOW CREATE TABLE")
ShowCreateTableCommand(name.asTableIdentifier)

case ShowCreateTableStatement(tbl, asSerde) if asSerde =>
val v1TableName = parseV1Table(tbl, "SHOW CREATE TABLE AS SERDE")
Expand Down Expand Up @@ -449,20 +438,27 @@ class ResolveSessionCatalog(
partitionSpec)

case ShowColumnsStatement(tbl, ns) =>
if (ns.isDefined && ns.get.length > 1) {
throw new AnalysisException(
s"Namespace name should have only one part if specified: ${ns.get.quoted}")
}
// Use namespace only if table name doesn't specify it. If namespace is already specified
// in the table name, it's checked against the given namespace below.
val nameParts = if (ns.isDefined && tbl.length == 1) {
ns.get ++ tbl
} else {
tbl
}
val sql = "SHOW COLUMNS"
val v1TableName = parseV1Table(tbl, sql).asTableIdentifier
val v1TableName = parseTempViewOrV1Table(nameParts, sql).asTableIdentifier
val resolver = conf.resolver
val db = ns match {
case Some(db) if (v1TableName.database.exists(!resolver(_, db.head))) =>
case Some(db) if v1TableName.database.exists(!resolver(_, db.head)) =>
throw new AnalysisException(
s"SHOW COLUMNS with conflicting databases: " +
s"'${db.head}' != '${v1TableName.database.get}'")
case _ => ns.map(_.head)
}
if (ns.isDefined && ns.get.length > 1) {
throw new AnalysisException(
s"Namespace name should have only one part if specified: ${ns.get.quoted}")
}
ShowColumnsCommand(db, v1TableName)

case AlterTableRecoverPartitionsStatement(tbl) =>
Expand Down Expand Up @@ -659,14 +655,7 @@ class ResolveSessionCatalog(
object SessionCatalogAndTable {
def unapply(nameParts: Seq[String]): Option[(CatalogPlugin, Seq[String])] = nameParts match {
case SessionCatalogAndIdentifier(catalog, ident) =>
if (nameParts.length == 1) {
// If there is only one name part, it means the current catalog is the session catalog.
// Here we return the original name part, to keep the error message unchanged for
// v1 commands.
Some(catalog -> nameParts)
} else {
Some(catalog -> ident.asMultipartIdentifier)
}
Some(catalog -> ident.asMultipartIdentifier)
case _ => None
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,14 +167,15 @@ class V2SessionCatalog(catalog: SessionCatalog, conf: SQLConf)
}

implicit class TableIdentifierHelper(ident: Identifier) {
import org.apache.spark.sql.connector.catalog.CatalogV2Implicits.IdentifierHelper

def asTableIdentifier: TableIdentifier = {
ident.namespace match {
case Array(db) =>
TableIdentifier(ident.name, Some(db))
case Array() =>
TableIdentifier(ident.name, Some(catalog.getCurrentDatabase))
case _ =>
throw new NoSuchTableException(ident)
throw new NoSuchTableException(
s"V2 session catalog requires a single-part namespace: ${ident.quoted}")
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,7 @@ struct<plan:string>
-- !query output
== Physical Plan ==
Execute DescribeColumnCommand
+- DescribeColumnCommand `t`, [b], false
+- DescribeColumnCommand `default`.`t`, [b], false


-- !query
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -879,7 +879,7 @@ Execute CreateViewCommand (1)
Output: []

(2) CreateViewCommand
Arguments: `explain_view`, SELECT key, val FROM explain_temp1, false, false, PersistedView
Arguments: `default`.`explain_view`, SELECT key, val FROM explain_temp1, false, false, PersistedView

(3) UnresolvedRelation
Arguments: [explain_temp1]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ CREATE VIEW v1_temp AS SELECT * FROM temp_table
struct<>
-- !query output
org.apache.spark.sql.AnalysisException
Not allowed to create a permanent view `v1_temp` by referencing a temporary view temp_table. Please create a temp view instead by CREATE TEMP VIEW;
Not allowed to create a permanent view `temp_view_test`.`v1_temp` by referencing a temporary view temp_table. Please create a temp view instead by CREATE TEMP VIEW;


-- !query
Expand Down Expand Up @@ -371,7 +371,7 @@ CREATE VIEW v4_temp AS
struct<>
-- !query output
org.apache.spark.sql.AnalysisException
Not allowed to create a permanent view `v4_temp` by referencing a temporary view temp_table. Please create a temp view instead by CREATE TEMP VIEW;
Not allowed to create a permanent view `temp_view_test`.`v4_temp` by referencing a temporary view temp_table. Please create a temp view instead by CREATE TEMP VIEW;


-- !query
Expand All @@ -383,7 +383,7 @@ CREATE VIEW v5_temp AS
struct<>
-- !query output
org.apache.spark.sql.AnalysisException
Not allowed to create a permanent view `v5_temp` by referencing a temporary view temp_table. Please create a temp view instead by CREATE TEMP VIEW;
Not allowed to create a permanent view `temp_view_test`.`v5_temp` by referencing a temporary view temp_table. Please create a temp view instead by CREATE TEMP VIEW;


-- !query
Expand Down Expand Up @@ -542,7 +542,7 @@ CREATE VIEW v6_temp AS SELECT * FROM base_table WHERE id IN (SELECT id FROM temp
struct<>
-- !query output
org.apache.spark.sql.AnalysisException
Not allowed to create a permanent view `v6_temp` by referencing a temporary view temp_table. Please create a temp view instead by CREATE TEMP VIEW;
Not allowed to create a permanent view `temp_view_test`.`v6_temp` by referencing a temporary view temp_table. Please create a temp view instead by CREATE TEMP VIEW;


-- !query
Expand All @@ -551,7 +551,7 @@ CREATE VIEW v7_temp AS SELECT t1.id, t2.a FROM base_table t1, (SELECT * FROM tem
struct<>
-- !query output
org.apache.spark.sql.AnalysisException
Not allowed to create a permanent view `v7_temp` by referencing a temporary view temp_table. Please create a temp view instead by CREATE TEMP VIEW;
Not allowed to create a permanent view `temp_view_test`.`v7_temp` by referencing a temporary view temp_table. Please create a temp view instead by CREATE TEMP VIEW;


-- !query
Expand All @@ -560,7 +560,7 @@ CREATE VIEW v8_temp AS SELECT * FROM base_table WHERE EXISTS (SELECT 1 FROM temp
struct<>
-- !query output
org.apache.spark.sql.AnalysisException
Not allowed to create a permanent view `v8_temp` by referencing a temporary view temp_table. Please create a temp view instead by CREATE TEMP VIEW;
Not allowed to create a permanent view `temp_view_test`.`v8_temp` by referencing a temporary view temp_table. Please create a temp view instead by CREATE TEMP VIEW;


-- !query
Expand All @@ -569,7 +569,7 @@ CREATE VIEW v9_temp AS SELECT * FROM base_table WHERE NOT EXISTS (SELECT 1 FROM
struct<>
-- !query output
org.apache.spark.sql.AnalysisException
Not allowed to create a permanent view `v9_temp` by referencing a temporary view temp_table. Please create a temp view instead by CREATE TEMP VIEW;
Not allowed to create a permanent view `temp_view_test`.`v9_temp` by referencing a temporary view temp_table. Please create a temp view instead by CREATE TEMP VIEW;


-- !query
Expand Down Expand Up @@ -678,7 +678,7 @@ CREATE VIEW temporal1 AS SELECT * FROM t1 CROSS JOIN tt
struct<>
-- !query output
org.apache.spark.sql.AnalysisException
Not allowed to create a permanent view `temporal1` by referencing a temporary view tt. Please create a temp view instead by CREATE TEMP VIEW;
Not allowed to create a permanent view `testviewschm2`.`temporal1` by referencing a temporary view tt. Please create a temp view instead by CREATE TEMP VIEW;


-- !query
Expand Down Expand Up @@ -719,7 +719,7 @@ CREATE VIEW temporal2 AS SELECT * FROM t1 INNER JOIN tt ON t1.num = tt.num2
struct<>
-- !query output
org.apache.spark.sql.AnalysisException
Not allowed to create a permanent view `temporal2` by referencing a temporary view tt. Please create a temp view instead by CREATE TEMP VIEW;
Not allowed to create a permanent view `testviewschm2`.`temporal2` by referencing a temporary view tt. Please create a temp view instead by CREATE TEMP VIEW;


-- !query
Expand Down Expand Up @@ -760,7 +760,7 @@ CREATE VIEW temporal3 AS SELECT * FROM t1 LEFT JOIN tt ON t1.num = tt.num2
struct<>
-- !query output
org.apache.spark.sql.AnalysisException
Not allowed to create a permanent view `temporal3` by referencing a temporary view tt. Please create a temp view instead by CREATE TEMP VIEW;
Not allowed to create a permanent view `testviewschm2`.`temporal3` by referencing a temporary view tt. Please create a temp view instead by CREATE TEMP VIEW;


-- !query
Expand Down Expand Up @@ -801,7 +801,7 @@ CREATE VIEW temporal4 AS SELECT * FROM t1 LEFT JOIN tt ON t1.num = tt.num2 AND t
struct<>
-- !query output
org.apache.spark.sql.AnalysisException
Not allowed to create a permanent view `temporal4` by referencing a temporary view tt. Please create a temp view instead by CREATE TEMP VIEW;
Not allowed to create a permanent view `testviewschm2`.`temporal4` by referencing a temporary view tt. Please create a temp view instead by CREATE TEMP VIEW;


-- !query
Expand All @@ -810,7 +810,7 @@ CREATE VIEW temporal5 AS SELECT * FROM t1 WHERE num IN (SELECT num FROM t1 WHERE
struct<>
-- !query output
org.apache.spark.sql.AnalysisException
Not allowed to create a permanent view `temporal5` by referencing a temporary view tt. Please create a temp view instead by CREATE TEMP VIEW;
Not allowed to create a permanent view `testviewschm2`.`temporal5` by referencing a temporary view tt. Please create a temp view instead by CREATE TEMP VIEW;


-- !query
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ SHOW CREATE TABLE tbl
-- !query schema
struct<createtab_stmt:string>
-- !query output
CREATE TABLE `tbl` (
CREATE TABLE `default`.`tbl` (
`a` INT,
`b` STRING,
`c` INT)
Expand Down Expand Up @@ -44,7 +44,7 @@ SHOW CREATE TABLE tbl
-- !query schema
struct<createtab_stmt:string>
-- !query output
CREATE TABLE `tbl` (
CREATE TABLE `default`.`tbl` (
`a` INT,
`b` STRING,
`c` INT)
Expand Down Expand Up @@ -75,7 +75,7 @@ SHOW CREATE TABLE tbl
-- !query schema
struct<createtab_stmt:string>
-- !query output
CREATE TABLE `tbl` (
CREATE TABLE `default`.`tbl` (
`a` INT,
`b` STRING,
`c` INT)
Expand Down Expand Up @@ -105,7 +105,7 @@ SHOW CREATE TABLE tbl
-- !query schema
struct<createtab_stmt:string>
-- !query output
CREATE TABLE `tbl` (
CREATE TABLE `default`.`tbl` (
`a` INT,
`b` STRING,
`c` INT)
Expand Down Expand Up @@ -135,7 +135,7 @@ SHOW CREATE TABLE tbl
-- !query schema
struct<createtab_stmt:string>
-- !query output
CREATE TABLE `tbl` (
CREATE TABLE `default`.`tbl` (
`b` STRING,
`c` INT,
`a` INT)
Expand Down Expand Up @@ -165,7 +165,7 @@ SHOW CREATE TABLE tbl
-- !query schema
struct<createtab_stmt:string>
-- !query output
CREATE TABLE `tbl` (
CREATE TABLE `default`.`tbl` (
`a` INT,
`b` STRING,
`c` INT)
Expand Down Expand Up @@ -197,7 +197,7 @@ SHOW CREATE TABLE tbl
-- !query schema
struct<createtab_stmt:string>
-- !query output
CREATE TABLE `tbl` (
CREATE TABLE `default`.`tbl` (
`a` INT,
`b` STRING,
`c` INT)
Expand Down Expand Up @@ -227,7 +227,7 @@ SHOW CREATE TABLE tbl
-- !query schema
struct<createtab_stmt:string>
-- !query output
CREATE TABLE `tbl` (
CREATE TABLE `default`.`tbl` (
`a` INT,
`b` STRING,
`c` INT)
Expand Down Expand Up @@ -257,7 +257,7 @@ SHOW CREATE TABLE tbl
-- !query schema
struct<createtab_stmt:string>
-- !query output
CREATE TABLE `tbl` (
CREATE TABLE `default`.`tbl` (
`a` FLOAT,
`b` DECIMAL(10,0),
`c` DECIMAL(10,0),
Expand Down Expand Up @@ -295,7 +295,7 @@ SHOW CREATE TABLE view_SPARK_30302 AS SERDE
-- !query schema
struct<createtab_stmt:string>
-- !query output
CREATE VIEW `view_SPARK_30302`(
CREATE VIEW `default`.`view_SPARK_30302`(
`aaa`,
`bbb`)
AS SELECT a, b FROM tbl
Expand Down Expand Up @@ -324,7 +324,7 @@ SHOW CREATE TABLE view_SPARK_30302 AS SERDE
-- !query schema
struct<createtab_stmt:string>
-- !query output
CREATE VIEW `view_SPARK_30302`(
CREATE VIEW `default`.`view_SPARK_30302`(
`aaa` COMMENT 'comment with \'quoted text\' for aaa',
`bbb`)
COMMENT 'This is a comment with \'quoted text\' for view'
Expand Down Expand Up @@ -354,7 +354,7 @@ SHOW CREATE TABLE view_SPARK_30302 AS SERDE
-- !query schema
struct<createtab_stmt:string>
-- !query output
CREATE VIEW `view_SPARK_30302`(
CREATE VIEW `default`.`view_SPARK_30302`(
`aaa`,
`bbb`)
TBLPROPERTIES (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ abstract class ShowCreateTableSuite extends QueryTest with SQLTestUtils {
val createTable = "CREATE TABLE `t1` (`a` STRUCT<`b`: STRING>)"
sql(s"$createTable USING json")
val shownDDL = getShowDDL("SHOW CREATE TABLE t1")
assert(shownDDL == createTable)
assert(shownDDL == "CREATE TABLE `default`.`t1` (`a` STRUCT<`b`: STRING>)")

checkCreateTable("t1")
}
Expand Down
Loading

0 comments on commit 9b86f36

Please sign in to comment.