[WIP] by dengzhhu653 · Pull Request #6450 · apache/hive

dengzhhu653 · 2026-04-24T00:20:07Z

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Copilot

Pull request overview

This PR appears to refactor the standalone metastore RawStore into pluggable sub-stores (e.g., TableStore, PrivilegeStore, NotificationStore) wired via a new @MetaDescriptor + unwrap() mechanism, with transaction/query lifecycle managed by proxy/handlers. It also updates tests/utilities to use the new store interfaces and improves a few test/metadata details (e.g., catalog name in stats).

Changes:

Introduces MetaDescriptor-based store unwrapping plus transactional proxying (TransactionHandler, RawStoreAware, PersistenceManagerProxy).
Adds new store interfaces + implementations for table/privilege/notification operations and migrates RawStore APIs to default delegations.
Updates several tests and utilities to use TableStore and a new DirectSqlConfigurator helper.

Reviewed changes

Copilot reviewed 20 out of 23 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/DirectSqlConfigurator.java	New test helper to toggle TRY_DIRECT_SQL and assert no unexpected direct SQL errors.
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestAlterPartitions.java	Updates expected exception types for rename-partition negative tests.
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/VerifyingObjectStore.java	Refactors verification paths to use `TableStore` + config toggling rather than internal SQL/ORM helpers.
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestObjectStore.java	Migrates tests to `TableStore` APIs and adds TRY_DIRECT_SQL toggling scopes.
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java	Removes unused imports.
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/StatisticsTestUtils.java	Ensures `ColumnStatisticsDesc` includes catalog name.
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/impl/PrivilegeStoreImpl.java	New privilege store implementation separated from `RawStore`.
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/impl/NotificationStoreImpl.java	New notification store implementation separated from `RawStore`.
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/iface/TableStore.java	New interface defining table/partition-related store APIs.
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/iface/PrivilegeStore.java	New interface defining privilege/role store APIs.
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/iface/NotificationStore.java	New interface defining notification event store APIs.
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/TransactionHandler.java	Dynamic-proxy invocation handler that opens/commits/rolls back transactions and closes tracked queries.
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/RawStoreAware.java	Base class to inject `RawStore` and `PersistenceManager` into store implementations.
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/PersistenceManagerProxy.java	Proxy wrapper to track opened queries and expose execution context via an auxiliary interface.
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/MetaDescriptor.java	New annotation used to discover store aliases and transactional behavior.
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/GetListHelper.java	New helper specialization for list-returning `GetHelper` use cases.
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metastore/GetHelper.java	New transaction-aware SQL-vs-ORM helper with direct SQL fallback behavior.
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/directsql/MetastoreDirectSqlUtils.java	Adjusts execution-context access to support proxied `PersistenceManager`.
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java	Removes/adjusts delegations in favor of new store unwrapping and adds missing database model import.
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java	Converts many APIs to default methods delegating to `unwrap()`ed stores; adds `unwrap(Class<T>)`.
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java	Moves partition-existence check earlier in alter-partition flow.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-24T00:39:56Z

+
+@MetaDescriptor(alias = "privilege", defaultImpl = PrivilegeStoreImpl.class)
+public interface PrivilegeStore {
+  boolean addRole(String rowName, String ownerName)


Parameter name rowName in addRole(String rowName, ...) looks like a typo (should be roleName). While it doesn't affect bytecode, it leaks into generated docs/IDE hints and is inconsistent with removeRole(String roleName) and other role APIs.

Suggested change

boolean addRole(String rowName, String ownerName)

boolean addRole(String roleName, String ownerName)

Copilot · 2026-04-24T00:39:57Z

+  default List<String> listPartitionNames(String catName, String db_name,
+      String tbl_name, short max_parts) throws MetaException {
+    try {
+      return unwrap(TableStore.class).listPartitionNames(new TableName(catName, db_name, tbl_name),
+          MetaStoreUtils.getDefaultCatalog(getConf()), null, null, max_parts);
+    } catch (NoSuchObjectException nse) {


listPartitionNames(catName, db_name, tbl_name, max_parts) passes MetaStoreUtils.getDefaultCatalog(getConf()) as the default partition name argument to TableStore.listPartitionNames. That value is a catalog name, not the default partition name, and will break partition-name pruning (e.g., it will treat the catalog string as the default partition value). Pass null (so TableStoreImpl uses MetastoreConf.DEFAULTPARTITIONNAME) or the actual default partition name instead.

Copilot · 2026-04-24T00:39:57Z

-      String catName, String dbName, String tableName, String columnName);
+  default List<HiveObjectPrivilege> listTableColumnGrantsAll(
+      String catName, String dbName, String tableName, String columnName) {
+    return unwrap(PrivilegeStore.class).listPartitionGrantsAll(new TableName(catName, dbName, tableName), columnName);


listTableColumnGrantsAll delegates to PrivilegeStore.listPartitionGrantsAll(...) with columnName passed as the partition name. This is the wrong underlying API and will return incorrect results (or throw) for table-column grants. Delegate to PrivilegeStore.listTableColumnGrantsAll(tableName, columnName) instead.

Suggested change

return unwrap(PrivilegeStore.class).listPartitionGrantsAll(new TableName(catName, dbName, tableName), columnName);

return unwrap(PrivilegeStore.class)

.listTableColumnGrantsAll(new TableName(catName, dbName, tableName), columnName);

Copilot · 2026-04-24T00:39:57Z

+  public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
+    // Redirect if the interface method "getExecutionContext" is called
+    if (method.getName().equals("getExecutionContext")) {
+      return getExecutionContext.bindTo(target).invokeWithArguments(args);


For no-arg methods on a dynamic proxy, args is null. Calling invokeWithArguments(args) will throw NPE when ExecutionContextReference.getExecutionContext() is invoked (e.g., from MetastoreDirectSqlUtils). Handle the null case by calling invokeWithArguments() with no arguments (or an empty array).

Suggested change

return getExecutionContext.bindTo(target).invokeWithArguments(args);

MethodHandle boundGetExecutionContext = getExecutionContext.bindTo(target);

return args == null ? boundGetExecutionContext.invokeWithArguments()

: boundGetExecutionContext.invokeWithArguments(args);

Copilot · 2026-04-24T00:39:58Z

+    case COLUMN:
+      Preconditions.checkArgument(objToRefresh.getColumnName()==null, "columnName must be null");
+      grants = getTableAllColumnGrants(catName, objToRefresh.getDbName(),
+          objToRefresh.getObjectName(), authorizer);
+      break;


In the COLUMN case, this asserts objToRefresh.getColumnName() == null, but HiveObjectRef.columnName is a required thrift field and callers (e.g., PrivilegeHandler) pass an actual column name. This will cause refresh_privileges to fail for column objects. Also, the code fetches all column grants for the table; it should load only the relevant column/partition-column grants based on objToRefresh (columnName + optional partValues).

sonarqubecloud · 2026-04-24T01:33:17Z

Quality Gate passed

Issues
553 New issues
0 Accepted issues

Measures
5 Security Hotspots
0.0% Coverage on New Code
2.2% Duplication on New Code

See analysis details on SonarQube Cloud

dengzhhu653 added 5 commits April 21, 2026 16:32

TableStore

bf3a13b

privilege store

515759d

minor

8f041f4

fix

bb18d88

ff

cac3461

asf-ci-hive added the tests pending label Apr 24, 2026

dengzhhu653 marked this pull request as draft April 24, 2026 00:20

dengzhhu653 requested a review from Copilot April 24, 2026 00:35

Copilot started reviewing on behalf of dengzhhu653 April 24, 2026 00:36 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

asf-ci-hive added tests failed and removed tests pending labels Apr 24, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP]#6450

[WIP]#6450
dengzhhu653 wants to merge 5 commits intoapache:masterfrom
dengzhhu653:split-objectstore

dengzhhu653 commented Apr 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

sonarqubecloud Bot commented Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	boolean addRole(String rowName, String ownerName)
	boolean addRole(String roleName, String ownerName)

	return unwrap(PrivilegeStore.class).listPartitionGrantsAll(new TableName(catName, dbName, tableName), columnName);
	return unwrap(PrivilegeStore.class)
	.listTableColumnGrantsAll(new TableName(catName, dbName, tableName), columnName);

-      return getExecutionContext.bindTo(target).invokeWithArguments(args);
+      MethodHandle boundGetExecutionContext = getExecutionContext.bindTo(target);
+      return args == null ? boundGetExecutionContext.invokeWithArguments()
+          : boundGetExecutionContext.invokeWithArguments(args);

Conversation

dengzhhu653 commented Apr 24, 2026

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Apr 24, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants