Core: Prevent dropping namespace when it contains views#14456
Conversation
64049b7 to
96047aa
Compare
| } | ||
|
|
||
| @Test | ||
| public void dropNonEmptyNamespace() { |
There was a problem hiding this comment.
Wouldn't it make sense to add this test case to other view catalogs (or at least to Nessie and Hive catalog)? Or maybe to a base class for those tests?
There was a problem hiding this comment.
@nandorKollar this is already the base class and all the other view catalog implementations run this set of tests
There was a problem hiding this comment.
Does drop namespace command support cascade?
There was a problem hiding this comment.
This patch LGTM, though I have doubt on detecting namespace empty in case of nested-namespace
lets say i have a structure like this
NS1
/ \ \
T V NS2
/ \
T V
T is Table, V is view
NS1 and NS2 a namespace
we want to drop NS1
shouldn't in addition to this we check
GET /namespaces?parent=NS1
This is probably a good candidate to tackle in a separate PR |
| List<TableIdentifier> viewIdentifiers = listViews(namespace); | ||
| if (viewIdentifiers != null && !viewIdentifiers.isEmpty()) { | ||
| throw new NamespaceNotEmptyException( | ||
| "Namespace %s is not empty. %s views exist.", namespace, viewIdentifiers.size()); |
There was a problem hiding this comment.
nit: shall we align this with the previous error message Namespace %s is not empty. Contains %d view(s).?
|
Shall we include this fix in 1.10.1? |
singhpk234
left a comment
There was a problem hiding this comment.
LGTM, too thanks @nastra !
| List<TableIdentifier> viewIdentifiers = listViews(namespace); | ||
| if (!viewIdentifiers.isEmpty()) { |
There was a problem hiding this comment.
out of curious, beside table and view, are there any other object can be inside namespace?
For instance: function?
There was a problem hiding this comment.
An Iceberg namespace can only hold tables/views. Functions/Procedures would only be relevant in the context of Spark and we have SparkCatalog that deals with those things
Co-authored-by: Prashant Singh <35593236+singhpk234@users.noreply.github.com>
|
thanks everyone for the reviews |
fixes #14175