ngsg commented on code in PR #5771:
URL: https://github.com/apache/hive/pull/5771#discussion_r2176269578
##########
ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java:
##########
@@ -142,2416 +40,28 @@
* so the readers of the objects in these maps should have the most recent
view of the object.
* But again, could be fragile.
*/
-public class SessionHiveMetaStoreClient extends
HiveMetaStoreClientWithLocalCache implements IMetaStoreClient {
- private static final Logger LOG =
LoggerFactory.getLogger(SessionHiveMetaStoreClient.class);
-
+public class SessionHiveMetaStoreClient extends BaseMetaStoreClientProxy
implements IMetaStoreClient {
SessionHiveMetaStoreClient(Configuration conf, Boolean allowEmbedded) throws
MetaException {
- super(conf, null, allowEmbedded);
+ super(createUnderlyingClient(conf, null, allowEmbedded));
}
SessionHiveMetaStoreClient(
Configuration conf, HiveMetaHookLoader hookLoader, Boolean
allowEmbedded) throws MetaException {
- super(conf, hookLoader, allowEmbedded);
- }
-
- private Warehouse wh = null;
-
- private Warehouse getWh() throws MetaException {
- if (wh == null) {
- wh = new Warehouse(conf);
- }
- return wh;
- }
-
- @Override
- protected void create_table(CreateTableRequest request) throws TException {
- org.apache.hadoop.hive.metastore.api.Table tbl = request.getTable();
- if (tbl.isTemporary()) {
- createTempTable(tbl);
- return;
- }
- super.create_table(request);
- }
-
- @Override
- protected void drop_table_with_environment_context(String catName, String
dbname, String name,
- boolean deleteData, EnvironmentContext envContext) throws TException,
UnsupportedOperationException {
- // First try temp table
- // TODO CAT - I think the right thing here is to always put temp tables in
the current
- // catalog. But we don't yet have a notion of current catalog, so we'll
have to hold on
- // until we do.
- org.apache.hadoop.hive.metastore.api.Table table = getTempTable(dbname,
name);
- if (table != null) {
- try {
- deleteTempTableColumnStatsForTable(dbname, name);
- } catch (NoSuchObjectException err){
- // No stats to delete, forgivable error.
- LOG.info(err.getMessage());
- }
- dropTempTable(table, deleteData, envContext);
- return;
- }
-
- // Try underlying client
- super.drop_table_with_environment_context(catName, dbname, name,
deleteData, envContext);
- }
-
- @Override
- public void truncateTable(String dbName, String tableName, List<String>
partNames) throws TException {
- // First try temp table
- org.apache.hadoop.hive.metastore.api.Table table = getTempTable(dbName,
tableName);
- if (table != null) {
- truncateTempTable(table);
- return;
- }
- // Try underlying client
- super.truncateTable(dbName, tableName, partNames);
- }
-
- @Override
- public void truncateTable(TableName tableName, List<String> partNames)
throws TException {
- // First try temp table
- org.apache.hadoop.hive.metastore.api.Table table =
getTempTable(tableName.getDb(), tableName.getTable());
- if (table != null) {
- truncateTempTable(table);
- return;
- }
- // Try underlying client
- super.truncateTable(tableName, partNames);
- }
-
- @Override
- public void truncateTable(String dbName, String tableName,
Review Comment:
`SessionHiveMetaStoreClient` and `HiveMetaStoreClientWithLocalCache` are
working as feature layers in the current codebase, and I understand that the
name `SessionHiveMetaStoreClient` could be used for
`SessionMetaStoreClientProxy` in this context. However, I think it is better to
keep the current naming for the following reasons:
1. In the new design, the session layer is not directly exposed. Instead,
there are two additional layers, `HookEnabledMetaStoreClientProxy` and
`SynchronizedMetaStoreClientProxy`, that wrap `SessionMetaStoreClientProxy`.
Therefore, keeping `SessionMetaStoreClientProxy` and
`SessionHiveMetaStoreClient` separate helps simplify the MetaStoreClient
creation logic.
2. More importantly, the purpose of this JIRA is to resolve the blocker of
HIVE-12679. HIVE-12679 aims to support third-party catalogs and is currently
blocked as there is no way to provide session-level features on top of them. To
address this issue, this JIRA proposes composable MetaStoreClients so that we
can compose a third-party catalog with a session-level feature layer. For this
reason, I believe it makes sense to introduce a separate proxy class for
session-level features, which reduces coupling with `ThriftHiveMetaStoreClient`.
Regarding session-level features, `SessionMetaStoreClientProxy` provides all
the existing functionality, including temporary table handling, transactions,
and session-level caching. As far as I know, the only API originally exposed by
`SessionHiveMetaStoreClient` is `getTempTablesForDatabase`, which remains
intact. If there are any features from `SessionHiveMetaStoreClient` that I may
have missed, I would appreciate it if you could point them out.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
