[hotfix] Fix drop database not working in terminal #3363
Conversation
MarigWeizhi
changed the title
Added test, PTAL |
| throw new NoSuchNamespaceException(namespace); | ||
| } | ||
| List<TableIDWithFormat> tables = unifiedCatalog.listTables(database); | ||
| if (!tables.isEmpty() && !cascade) { |
There was a problem hiding this comment.
Why not drop all tables first and then drop the database?
I think should this logic be implemented directly by the parent (spark-common).
We already have concrete logic in SparkUnifiedCatalogBase.dropNamespace.
so，remove Override is enough.
WDYT?
There was a problem hiding this comment.
I agreed that we may reuse some implementation in the parent class. However I think it is dangerous to delete all tables by default in Spark 3.2 implementation. So we may:
- Add a method in the base class to support dropping namespace cascade or not.
- Drop namespace by default with false for Spark 3.2
|
Thanks for your review @zhoujinsong @huyuanfeng2018 |
...ark/v3.3/amoro-mixed-spark-3.3/src/main/java/org/apache/amoro/spark/SparkUnifiedCatalog.java
Outdated
Show resolved
Hide resolved
|
Thanks for the contribution! @MarigWeizhi |
Why are the changes needed?
Close #xxx.
Brief change log
dropNamespaceto support cascade or non-cascade inSparkUnifiedCatalogBasefor spark 3.3How was this patch tested?
Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before making a pull request
Documentation