-
Notifications
You must be signed in to change notification settings - Fork 29.3k
[SPARK-19917][SQL]qualified partition path stored in catalog #17254
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.
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
Changes from 4 commits
f59d65f
99a3c53
f61af0a
c028c75
fadc15c
e168b34
f8ab46a
20e0168
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -353,9 +353,20 @@ class SessionCatalog( | |
| val db = formatDatabaseName(tableDefinition.identifier.database.getOrElse(getCurrentDatabase)) | ||
| val table = formatTableName(tableDefinition.identifier.table) | ||
| val tableIdentifier = TableIdentifier(table, Some(db)) | ||
| val newTableDefinition = tableDefinition.copy(identifier = tableIdentifier) | ||
| requireDbExists(db) | ||
| requireTableExists(tableIdentifier) | ||
| val newTableDefinition = if (tableDefinition.storage.locationUri.isDefined | ||
| && !tableDefinition.storage.locationUri.get.isAbsolute) { | ||
| // make the location of the table qualified. | ||
| val qualifiedTableLocation = | ||
| makeQualifiedPath(tableDefinition.storage.locationUri.get) | ||
| tableDefinition.copy( | ||
| storage = tableDefinition.storage.copy(locationUri = Some(qualifiedTableLocation)), | ||
| identifier = tableIdentifier) | ||
| } else { | ||
| tableDefinition.copy(identifier = tableIdentifier) | ||
| } | ||
|
|
||
| externalCatalog.alterTable(newTableDefinition) | ||
| } | ||
|
|
||
|
|
@@ -882,7 +893,8 @@ class SessionCatalog( | |
| requireTableExists(TableIdentifier(table, Option(db))) | ||
| requireExactMatchedPartitionSpec(parts.map(_.spec), getTableMetadata(tableName)) | ||
| requireNonEmptyValueInPartitionSpec(parts.map(_.spec)) | ||
| externalCatalog.createPartitions(db, table, parts, ignoreIfExists) | ||
| externalCatalog.createPartitions( | ||
| db, table, partitionWithQualifiedPath(tableName, parts), ignoreIfExists) | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -942,7 +954,8 @@ class SessionCatalog( | |
| requireTableExists(TableIdentifier(table, Option(db))) | ||
| requireExactMatchedPartitionSpec(parts.map(_.spec), getTableMetadata(tableName)) | ||
| requireNonEmptyValueInPartitionSpec(parts.map(_.spec)) | ||
| externalCatalog.alterPartitions(db, table, parts) | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unnecessary blank line |
||
| externalCatalog.alterPartitions(db, table, partitionWithQualifiedPath(tableName, parts)) | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -1064,6 +1077,23 @@ class SessionCatalog( | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Make the partition path qualified. | ||
| * If the partition path is relative, e.g. 'paris', it will be qualified with | ||
| * parent path using table location, e.g. 'file:/warehouse/table/paris' | ||
| */ | ||
| private def partitionWithQualifiedPath( | ||
|
windpiger marked this conversation as resolved.
|
||
| tableIdentifier: TableIdentifier, | ||
| parts: Seq[CatalogTablePartition]): Seq[CatalogTablePartition] = { | ||
| parts.map { part => | ||
| if (part.storage.locationUri.isDefined && !part.storage.locationUri.get.isAbsolute) { | ||
| val tbl = getTableMetadata(tableIdentifier) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should get table metadata only once.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can mark it as lazy val in case we don't need to qualify any path. |
||
| val partPath = new Path(new Path(tbl.location), new Path(part.storage.locationUri.get)) | ||
| val qualifiedPartPath = makeQualifiedPath(CatalogUtils.stringToURI(partPath.toString)) | ||
| part.copy(storage = part.storage.copy(locationUri = Some(qualifiedPartPath))) | ||
| } else part | ||
| } | ||
| } | ||
| // ---------------------------------------------------------------------------- | ||
| // Functions | ||
| // ---------------------------------------------------------------------------- | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1372,7 +1372,8 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils { | |
| // if (isUsingHiveMetastore) { | ||
| // assert(storageFormat.properties.get("path") === expected) | ||
| // } | ||
| assert(storageFormat.locationUri.map(_.getPath) === Some(expected.getPath)) | ||
| assert(storageFormat.locationUri === | ||
| Some(makeQualifiedPath(CatalogUtils.URIToString(expected)))) | ||
| } | ||
| // set table location | ||
| sql("ALTER TABLE dbx.tab1 SET LOCATION '/path/to/your/lovely/heart'") | ||
|
|
@@ -1386,7 +1387,9 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils { | |
| verifyLocation(new URI("/swanky/steak/place")) | ||
| // set table partition location without explicitly specifying database | ||
| sql("ALTER TABLE tab1 PARTITION (a='1', b='2') SET LOCATION 'vienna'") | ||
| verifyLocation(new URI("vienna"), Some(partSpec)) | ||
| val table = spark.sessionState.catalog.getTableMetadata(TableIdentifier("tab1")) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ALTER TABLE PARTITION SET LOCATION |
||
| val viennaPartPath = new Path(new Path(table. location), "vienna") | ||
| verifyLocation(CatalogUtils.stringToURI(viennaPartPath.toString), Some(partSpec)) | ||
| // table to alter does not exist | ||
| intercept[AnalysisException] { | ||
| sql("ALTER TABLE dbx.does_not_exist SET LOCATION '/mister/spark'") | ||
|
|
@@ -1550,13 +1553,11 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils { | |
| "PARTITION (a='2', b='6') LOCATION 'paris' PARTITION (a='3', b='7')") | ||
| assert(catalog.listPartitions(tableIdent).map(_.spec).toSet == Set(part1, part2, part3)) | ||
| assert(catalog.getPartition(tableIdent, part1).storage.locationUri.isDefined) | ||
| val partitionLocation = if (isUsingHiveMetastore) { | ||
| val tableLocation = catalog.getTableMetadata(tableIdent).storage.locationUri | ||
| assert(tableLocation.isDefined) | ||
| makeQualifiedPath(new Path(tableLocation.get.toString, "paris").toString) | ||
| } else { | ||
| new URI("paris") | ||
| } | ||
|
|
||
| val tableLocation = catalog.getTableMetadata(tableIdent).storage.locationUri | ||
| assert(tableLocation.isDefined) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ALTER TABLE ADD PARTITION LOCATION now InMemoryCatalog has the same action with HiveExternalCatalog |
||
| val partitionLocation = makeQualifiedPath( | ||
| new Path(tableLocation.get.toString, "paris").toString) | ||
|
|
||
| assert(catalog.getPartition(tableIdent, part2).storage.locationUri == Option(partitionLocation)) | ||
| assert(catalog.getPartition(tableIdent, part3).storage.locationUri.isDefined) | ||
|
|
@@ -2138,7 +2139,7 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils { | |
| spark.sessionState.catalog.refreshTable(TableIdentifier("t")) | ||
|
|
||
| val table1 = spark.sessionState.catalog.getTableMetadata(TableIdentifier("t")) | ||
| assert(table1.location == newDir) | ||
| assert(table1.location == makeQualifiedPath(newDir.toString)) | ||
| assert(!newDirFile.exists) | ||
|
|
||
| spark.sql("INSERT INTO TABLE t SELECT 'c', 1") | ||
|
|
@@ -2501,6 +2502,13 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils { | |
| assert(table.location.toString.startsWith("file:/")) | ||
| } | ||
|
|
||
| withTempDir { dir => | ||
| assert(!dir.getAbsolutePath.startsWith("file:/")) | ||
| spark.sql(s"ALTER TABLE t SET LOCATION '$dir'") | ||
| val table = spark.sessionState.catalog.getTableMetadata(TableIdentifier("t")) | ||
| assert(table.location.toString.startsWith("file:/")) | ||
| } | ||
|
|
||
| withTempDir { dir => | ||
| assert(!dir.getAbsolutePath.startsWith("file:/")) | ||
| // The parser does not recognize the backslashes on Windows as they are. | ||
|
|
@@ -2519,6 +2527,37 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils { | |
| } | ||
| } | ||
|
|
||
| test("the qualified path of a partition is stored in the catalog") { | ||
| withTable("t") { | ||
| withTempDir { dir => | ||
| spark.sql( | ||
| s""" | ||
| |CREATE TABLE t(a STRING, b STRING) USING parquet PARTITIONED BY(b) LOCATION '$dir' | ||
| """.stripMargin) | ||
| spark.sql("INSERT INTO TABLE t PARTITION(b=1) SELECT 2") | ||
| val part = spark.sessionState.catalog.getPartition(TableIdentifier("t"), Map("b" -> "1")) | ||
| assert(part.storage.locationUri.contains( | ||
| makeQualifiedPath(new File(dir, "b=1").getAbsolutePath))) | ||
| assert(part.storage.locationUri.get.toString.startsWith("file:/")) | ||
| } | ||
| withTempDir { dir => | ||
| spark.sql(s"ALTER TABLE t PARTITION(b=1) SET LOCATION '$dir'") | ||
|
|
||
| val table = spark.sessionState.catalog.getTableMetadata(TableIdentifier("t")) | ||
|
windpiger marked this conversation as resolved.
Outdated
|
||
| val part = spark.sessionState.catalog.getPartition(TableIdentifier("t"), Map("b" -> "1")) | ||
| assert(part.storage.locationUri.contains(makeQualifiedPath(dir.getAbsolutePath))) | ||
| assert(part.storage.locationUri.get.toString.startsWith("file:/")) | ||
| } | ||
|
|
||
| withTempDir { dir => | ||
| spark.sql(s"ALTER TABLE t ADD PARTITION(b=2) LOCATION '$dir'") | ||
| val part = spark.sessionState.catalog.getPartition(TableIdentifier("t"), Map("b" -> "2")) | ||
| assert(part.storage.locationUri.contains(makeQualifiedPath(dir.getAbsolutePath))) | ||
| assert(part.storage.locationUri.get.toString.startsWith("file:/")) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| protected def testAddColumn(provider: String): Unit = { | ||
| withTable("t1") { | ||
| sql(s"CREATE TABLE t1 (c1 int) USING $provider") | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: 2 space indentation