HIVE-29423: Move delete_column_statistics_req out from HMSHandler - #6661
HIVE-29423: Move delete_column_statistics_req out from HMSHandler#6661rtrivedi12 wants to merge 1 commit into
Conversation
|
There was a problem hiding this comment.
Pull request overview
This PR continues the metastore handler decomposition by extracting the delete_column_statistics_req implementation out of HMSHandler into a dedicated DeleteColumnStatisticsHandler based on the existing @RequestHandler / AbstractRequestHandler framework.
Changes:
- Refactors
HMSHandler.delete_column_statistics_req()to delegate execution toAbstractRequestHandler.offer(this, req)while preserving audit start/end behavior and standard exception translation. - Adds
DeleteColumnStatisticsHandlerto implement table- and partition-level column statistics deletion, including transactional listener notifications and post-commit event publication.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java | Delegates delete_column_statistics_req to the request-handler framework and routes exceptions through standard handling while ensuring endFunction receives any thrown exception. |
| standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/DeleteColumnStatisticsHandler.java | New request handler implementing the delete column statistics logic formerly in HMSHandler, including transaction and listener event logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @Override | ||
| protected void afterExecute(DeleteColumnStatisticsResult result) throws TException, IOException { | ||
| if (result != null && !handler.getListeners().isEmpty()) { | ||
| for (ListenerEvent event : result.events()) { | ||
| MetaStoreListenerNotifier.notifyEvent(handler.getTransactionalListeners(), result.eventType(), event); | ||
| } | ||
| } | ||
| super.afterExecute(result); | ||
| } |
There was a problem hiding this comment.
Please take care of this typo fix
| return ret; | ||
| } catch (Exception e) { | ||
| ex = e; | ||
| throw handleException(e).throwIfInstance(MetaException.class, NoSuchObjectException.class) |
There was a problem hiding this comment.
nit: can we just transform the exception to TException? throw handleException(e).defaultTException()
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/DeleteColumnStatisticsHandler.java:154
- Post-commit listener notification is using the transactional listener list. Transactional listeners are already notified during execution; after commit this should notify the regular listeners (handler.getListeners()). As written, non-transactional listeners will never receive DELETE_*_COLUMN_STAT events and transactional listeners may receive duplicate notifications.
protected void afterExecute(DeleteColumnStatisticsResult result) throws TException, IOException {
if (result != null && !handler.getListeners().isEmpty()) {
for (ListenerEvent event : result.events()) {
MetaStoreListenerNotifier.notifyEvent(handler.getTransactionalListeners(), result.eventType(), event);
}
}
What changes were proposed in this pull request?
Extract
delete_column_statistics_reqlogic fromHMSHandlerinto a newDeleteColumnStatisticsHandler, following the existing@RequestHandler/AbstractRequestHandlerDeleteColumnStatisticsHandlerfor table- and partition-level column statistics deletion, including transactional listener events and post-commit notification.HMSHandler.delete_column_statistics_req()used to audit logging (startFunction/endFunction) and delegation viaAbstractRequestHandler.offer(this, req).Why are the changes needed?
HMSHandleris being decomposed into focused request handlers (HIVE-27224).Does this PR introduce any user-facing change?
No
How was this patch tested?
mvn test -pl standalone-metastore/metastore-server -am
-Dtest=TestStats,TestStatisticsManagement