pythaac commented on code in PR #10301:
URL: https://github.com/apache/gravitino/pull/10301#discussion_r2906474094
##########
core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java:
##########
@@ -402,7 +402,6 @@ public boolean purgeTable(NameIdentifier ident) throws
UnsupportedOperationExcep
store.delete(ident, TABLE);
} catch (NoSuchEntityException e) {
LOG.warn("The table to be purged does not exist in the store: {}",
ident, e);
- return false;
Review Comment:
Thanks for pointing this out, @yuqi1129 ! It is a very valid point regarding
the ambiguity of the term "table" in the method signature🙂
After reviewing the context, here is my understanding:
1. Both `dropTable` and `purgeTable` are designed to delete the entity from
both the Gravitino store and the underlying catalog, meaning the "table" refers
to both of them.
2. However, both methods return the deletion result based **only from the
underlying catalog**, regardless of whether the table exists in the Gravitino
store.
3. As a result, the current Javadoc(`... false if the table does not exist`)
is ambiguous. It doesn't specify whether "table" refers to the entity in the
Gravitino store or the underlying catalog.
To resolve this ambiguity, I think we should update the javadoc as follows:
- `dropTable` : True if the table is dropped, false if the table does not
exist `in the underlying catalog`.
- `purgeTable` : True if the table is purged, false if the table does not
exist `in the underlying catalog`.
If you agree with this approach, I will update them in this PR. Please Let
me know what you think!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
