Skip to content

Commit

Permalink
[SPARK-33364][SQL][FOLLOWUP] Refine the catalog v2 API to purge a table
Browse files Browse the repository at this point in the history
### What changes were proposed in this pull request?

This is a followup of #30267

Inspired by #30886, it's better to have 2 methods `def dropTable` and `def purgeTable`, than `def dropTable(ident)` and `def dropTable(ident, purge)`.

### Why are the changes needed?

1. make the APIs orthogonal. Previously, `def dropTable(ident, purge)` calls `def dropTable(ident)` and is a superset.
2. simplifies the catalog implementation a little bit. Now the `if (purge) ... else ...` check is done at the Spark side.

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

No.

### How was this patch tested?

existing tests

Closes #30890 from cloud-fan/purgeTable.

Authored-by: Wenchen Fan <[email protected]>
Signed-off-by: HyukjinKwon <[email protected]>
  • Loading branch information
cloud-fan authored and HyukjinKwon committed Dec 23, 2020
1 parent 303b8c8 commit ec1560a
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,11 @@ public boolean dropTable(Identifier ident) {
return asTableCatalog().dropTable(ident);
}

@Override
public boolean purgeTable(Identifier ident) {
return asTableCatalog().purgeTable(ident);
}

@Override
public void renameTable(
Identifier oldIdent,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,26 +173,23 @@ Table alterTable(
boolean dropTable(Identifier ident);

/**
* Drop a table in the catalog with an option to purge.
* Drop a table in the catalog and completely remove its data by skipping a trash even if it is
* supported.
* <p>
* If the catalog supports views and contains a view for the identifier and not a table, this
* must not drop the view and must return false.
* <p>
* If the catalog supports the option to purge a table, this method must be overridden.
* The default implementation falls back to {@link #dropTable(Identifier)} dropTable} if the
* purge option is set to false. Otherwise, it throws {@link UnsupportedOperationException}.
* If the catalog supports to purge a table, this method should be overridden.
* The default implementation throws {@link UnsupportedOperationException}.
*
* @param ident a table identifier
* @param purge whether a table should be purged
* @return true if a table was deleted, false if no table exists for the identifier
* @throws UnsupportedOperationException If table purging is not supported
*
* @since 3.1.0
*/
default boolean dropTable(Identifier ident, boolean purge) {
if (purge) {
throw new UnsupportedOperationException("Purge option is not supported.");
}
return dropTable(ident);
default boolean purgeTable(Identifier ident) throws UnsupportedOperationException {
throw new UnsupportedOperationException("Purge table is not supported.");
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,11 @@ class TableCatalogSuite extends SparkFunSuite {
assert(!catalog.tableExists(testIdent))
}

test("purgeTable") {
val catalog = newCatalog()
intercept[UnsupportedOperationException](catalog.purgeTable(testIdent))
}

test("renameTable") {
val catalog = newCatalog()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ case class DropTableExec(
override def run(): Seq[InternalRow] = {
if (catalog.tableExists(ident)) {
invalidateCache()
catalog.dropTable(ident, purge)
if (purge) catalog.purgeTable(ident) else catalog.dropTable(ident)
} else if (!ifExists) {
throw new NoSuchTableException(ident)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ class DropTableSuite extends command.DropTableSuiteBase with CommandSuiteBase {
val errMsg = intercept[UnsupportedOperationException] {
sql(s"DROP TABLE $catalog.ns.tbl PURGE")
}.getMessage
// The default TableCatalog.dropTable implementation doesn't support the purge option.
assert(errMsg.contains("Purge option is not supported"))
// The default TableCatalog.purgeTable implementation throws an exception.
assert(errMsg.contains("Purge table is not supported"))
}
}

Expand Down

0 comments on commit ec1560a

Please sign in to comment.