Copilot commented on code in PR #6427:
URL: https://github.com/apache/hive/pull/6427#discussion_r3070568071
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1469,80 +1436,18 @@ public List<ExtendedTableInfo> get_tables_ext(final
GetTablesExtRequest req) thr
return ret;
}
- private ExtendedTableInfo convertTableToExtendedTable (Table table,
- List<String>
processorCapabilities, int mask) {
- ExtendedTableInfo extTable = new ExtendedTableInfo(table.getTableName());
- if ((mask & GetTablesExtRequestFields.ACCESS_TYPE.getValue()) ==
GetTablesExtRequestFields.ACCESS_TYPE.getValue()) {
- extTable.setAccessType(table.getAccessType());
- }
-
- if ((mask & GetTablesExtRequestFields.PROCESSOR_CAPABILITIES.getValue())
- == GetTablesExtRequestFields.PROCESSOR_CAPABILITIES.getValue()) {
-
extTable.setRequiredReadCapabilities(table.getRequiredReadCapabilities());
-
extTable.setRequiredWriteCapabilities(table.getRequiredWriteCapabilities());
- }
-
- return extTable;
- }
-
@Override
public GetTableResult get_table_req(GetTableRequest req) throws
MetaException,
NoSuchObjectException {
req.setCatName(req.isSetCatName() ? req.getCatName() :
getDefaultCatalog(conf));
- return new GetTableResult(getTableInternal(req));
- }
-
- /**
- * This function retrieves table from metastore. If getColumnStats flag is
true,
- * then engine should be specified so the table is retrieve with the column
stats
- * for that engine.
- */
- private Table getTableInternal(GetTableRequest getTableRequest) throws
MetaException, NoSuchObjectException {
-
- Preconditions.checkArgument(!getTableRequest.isGetColumnStats() ||
getTableRequest.getEngine() != null,
- "To retrieve column statistics with a table, engine parameter cannot
be null");
-
- if (isInTest) {
- assertClientHasCapability(getTableRequest.getCapabilities(),
ClientCapability.TEST_CAPABILITY, "Hive tests",
- "get_table_req");
- }
-
+ startTableFunction("get_table", req.getCatName(), req.getDbName(),
req.getTblName());
Table t = null;
- startTableFunction("get_table", getTableRequest.getCatName(),
getTableRequest.getDbName(),
- getTableRequest.getTblName());
- Exception ex = null;
try {
- t = get_table_core(getTableRequest);
- if (MetaStoreUtils.isInsertOnlyTableParam(t.getParameters())) {
- assertClientHasCapability(getTableRequest.getCapabilities(),
ClientCapability.INSERT_ONLY_TABLES,
- "insert-only tables", "get_table_req");
- }
-
- if (CollectionUtils.isEmpty(getTableRequest.getProcessorCapabilities())
|| getTableRequest
- .getProcessorCapabilities().contains("MANAGERAWMETADATA")) {
- LOG.info("Skipping translation for processor with " +
getTableRequest.getProcessorIdentifier());
- } else {
- if (transformer != null) {
- List<Table> tList = new ArrayList<>();
- tList.add(t);
- Map<Table, List<String>> ret = transformer
- .transform(tList, getTableRequest.getProcessorCapabilities(),
getTableRequest.getProcessorIdentifier());
- if (ret.size() > 1) {
- LOG.warn("Unexpected resultset size:" + ret.size());
- throw new MetaException("Unexpected result from metadata
transformer:return list size is " + ret.size());
- }
- t = ret.keySet().iterator().next();
- }
- }
-
- firePreEvent(new PreReadTableEvent(t, this));
- } catch (MetaException | NoSuchObjectException e) {
- ex = e;
- throw e;
+ t = GetTableHandler.getTable(this, req, false);
} finally {
- endFunction("get_table", t != null, ex, getTableRequest.getTblName());
+ endFunction("get_table", t != null, null, req.getTblName());
}
Review Comment:
endFunction is always invoked with a null exception here. If
GetTableHandler.getTable throws, listeners/metrics lose the failure cause
(BaseHandler.endFunction passes the exception into
MetaStoreEndFunctionContext). Capture the thrown exception (as in other
HMSHandler methods) and pass it to endFunction so failures are correctly
reported.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/GetTableHandler.java:
##########
@@ -0,0 +1,490 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore.handler;
+
+import com.google.common.base.Preconditions;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.common.StatsSetupConst;
+import org.apache.hadoop.hive.common.TableName;
+import org.apache.hadoop.hive.metastore.HMSHandler;
+import org.apache.hadoop.hive.metastore.IHMSHandler;
+import org.apache.hadoop.hive.metastore.IMetaStoreMetadataTransformer;
+import org.apache.hadoop.hive.metastore.MetaStoreFilterHook;
+import org.apache.hadoop.hive.metastore.RawStore;
+import org.apache.hadoop.hive.metastore.TableType;
+import org.apache.hadoop.hive.metastore.api.ClientCapabilities;
+import org.apache.hadoop.hive.metastore.api.ClientCapability;
+import org.apache.hadoop.hive.metastore.api.ColumnStatistics;
+import org.apache.hadoop.hive.metastore.api.Database;
+import org.apache.hadoop.hive.metastore.api.ExtendedTableInfo;
+import org.apache.hadoop.hive.metastore.api.GetProjectionsSpec;
+import org.apache.hadoop.hive.metastore.api.GetTableRequest;
+import org.apache.hadoop.hive.metastore.api.GetTablesExtRequest;
+import org.apache.hadoop.hive.metastore.api.GetTablesExtRequestFields;
+import org.apache.hadoop.hive.metastore.api.GetTablesRequest;
+import org.apache.hadoop.hive.metastore.api.InvalidOperationException;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.api.NoSuchObjectException;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.hadoop.hive.metastore.api.TableMeta;
+import org.apache.hadoop.hive.metastore.api.UnknownDBException;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import
org.apache.hadoop.hive.metastore.dataconnector.DataConnectorProviderFactory;
+import org.apache.hadoop.hive.metastore.events.PreReadTableEvent;
+import org.apache.hadoop.hive.metastore.utils.FilterUtils;
+import org.apache.hadoop.hive.metastore.utils.MetaStoreUtils;
+import org.apache.thrift.TException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static
org.apache.hadoop.hive.metastore.ExceptionHandler.handleException;
+import static
org.apache.hadoop.hive.metastore.ExceptionHandler.newMetaException;
+import static
org.apache.hadoop.hive.metastore.conf.MetastoreConf.ConfVars.HIVE_IN_TEST;
+import static org.apache.hadoop.hive.metastore.utils.MetaStoreUtils.CAT_NAME;
+import static org.apache.hadoop.hive.metastore.utils.MetaStoreUtils.DB_NAME;
+import static
org.apache.hadoop.hive.metastore.utils.MetaStoreUtils.getDefaultCatalog;
+import static
org.apache.hadoop.hive.metastore.utils.MetaStoreUtils.isDatabaseRemote;
+import static
org.apache.hadoop.hive.metastore.utils.MetaStoreUtils.parseDbName;
+import static
org.apache.hadoop.hive.metastore.utils.StringUtils.normalizeIdentifier;
+
+@SuppressWarnings({"unchecked", "rawtypes"})
+@RequestHandler(requestBody = GetTableHandler.GetTableReq.class)
+public class GetTableHandler<R, T> extends
+ AbstractRequestHandler<GetTableHandler.GetTableReq<R>,
GetTableHandler.GetTableResult<T>> {
+ private static final Logger LOG =
LoggerFactory.getLogger(GetTableHandler.class);
+ private RawStore ms;
+ private IMetaStoreMetadataTransformer transformer;
+ private MetaStoreFilterHook filterHook;
+ private Configuration conf;
+ private boolean isInTest;
+ GetTableHandler(IHMSHandler handler, boolean async, GetTableReq request) {
+ super(handler, async, request);
+ }
+
Review Comment:
GetTableHandler declares a constructor `GetTableHandler(IHMSHandler,
boolean, GetTableReq)` but AbstractRequestHandler’s factory/validation requires
a `(IHMSHandler, requestBody)` constructor (it reflects
`clz.getDeclaredConstructor(IHMSHandler.class, requestBody)` and instantiates
with exactly those 2 args). As written, `AbstractRequestHandler.offer(...)`
will fail to instantiate this handler at runtime. Add a 2-arg constructor
matching `(IHMSHandler, GetTableReq)` (and move async decision into
`super(...)`), or change the constructor to match the expected signature.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/GetTableHandler.java:
##########
@@ -0,0 +1,490 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore.handler;
+
+import com.google.common.base.Preconditions;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.common.StatsSetupConst;
+import org.apache.hadoop.hive.common.TableName;
+import org.apache.hadoop.hive.metastore.HMSHandler;
+import org.apache.hadoop.hive.metastore.IHMSHandler;
+import org.apache.hadoop.hive.metastore.IMetaStoreMetadataTransformer;
+import org.apache.hadoop.hive.metastore.MetaStoreFilterHook;
+import org.apache.hadoop.hive.metastore.RawStore;
+import org.apache.hadoop.hive.metastore.TableType;
+import org.apache.hadoop.hive.metastore.api.ClientCapabilities;
+import org.apache.hadoop.hive.metastore.api.ClientCapability;
+import org.apache.hadoop.hive.metastore.api.ColumnStatistics;
+import org.apache.hadoop.hive.metastore.api.Database;
+import org.apache.hadoop.hive.metastore.api.ExtendedTableInfo;
+import org.apache.hadoop.hive.metastore.api.GetProjectionsSpec;
+import org.apache.hadoop.hive.metastore.api.GetTableRequest;
+import org.apache.hadoop.hive.metastore.api.GetTablesExtRequest;
+import org.apache.hadoop.hive.metastore.api.GetTablesExtRequestFields;
+import org.apache.hadoop.hive.metastore.api.GetTablesRequest;
+import org.apache.hadoop.hive.metastore.api.InvalidOperationException;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.api.NoSuchObjectException;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.hadoop.hive.metastore.api.TableMeta;
+import org.apache.hadoop.hive.metastore.api.UnknownDBException;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import
org.apache.hadoop.hive.metastore.dataconnector.DataConnectorProviderFactory;
+import org.apache.hadoop.hive.metastore.events.PreReadTableEvent;
+import org.apache.hadoop.hive.metastore.utils.FilterUtils;
+import org.apache.hadoop.hive.metastore.utils.MetaStoreUtils;
+import org.apache.thrift.TException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static
org.apache.hadoop.hive.metastore.ExceptionHandler.handleException;
+import static
org.apache.hadoop.hive.metastore.ExceptionHandler.newMetaException;
+import static
org.apache.hadoop.hive.metastore.conf.MetastoreConf.ConfVars.HIVE_IN_TEST;
+import static org.apache.hadoop.hive.metastore.utils.MetaStoreUtils.CAT_NAME;
+import static org.apache.hadoop.hive.metastore.utils.MetaStoreUtils.DB_NAME;
+import static
org.apache.hadoop.hive.metastore.utils.MetaStoreUtils.getDefaultCatalog;
+import static
org.apache.hadoop.hive.metastore.utils.MetaStoreUtils.isDatabaseRemote;
+import static
org.apache.hadoop.hive.metastore.utils.MetaStoreUtils.parseDbName;
+import static
org.apache.hadoop.hive.metastore.utils.StringUtils.normalizeIdentifier;
+
+@SuppressWarnings({"unchecked", "rawtypes"})
+@RequestHandler(requestBody = GetTableHandler.GetTableReq.class)
+public class GetTableHandler<R, T> extends
+ AbstractRequestHandler<GetTableHandler.GetTableReq<R>,
GetTableHandler.GetTableResult<T>> {
+ private static final Logger LOG =
LoggerFactory.getLogger(GetTableHandler.class);
+ private RawStore ms;
+ private IMetaStoreMetadataTransformer transformer;
+ private MetaStoreFilterHook filterHook;
+ private Configuration conf;
+ private boolean isInTest;
+ GetTableHandler(IHMSHandler handler, boolean async, GetTableReq request) {
+ super(handler, async, request);
+ }
+
+ @Override
+ protected void beforeExecute() throws TException, IOException {
+ this.ms = handler.getMS();
+ this.transformer = handler.getMetadataTransformer();
+ this.filterHook = handler.getMetaFilterHook();
+ this.conf = handler.getConf();
+
+ this.isInTest = MetastoreConf.getBoolVar(handler.getConf(), HIVE_IN_TEST);
+ }
+
+ @Override
+ protected GetTableResult execute() throws TException, IOException {
+ R req = request.getRequest();
+ if (req instanceof GetTablesExtRequest getTablesExtRequest) {
+ List<ExtendedTableInfo> extendedTableInfos =
getTablesExt(getTablesExtRequest);
+ return new GetTableResult(extendedTableInfos, true);
+ } else if (req instanceof GetTableRequest getTableRequest) {
+ Table table = request.rawTable ? getTableCore(getTableRequest) :
getTable(getTableRequest);
+ return new GetTableResult(List.of(table), true);
+ } else if (req instanceof GetTablesRequest getTablesRequest) {
+ List<Table> tables = getTableObjects(getTablesRequest);
+ return new GetTableResult(tables, true);
+ } else if (req instanceof GetTableNamesRequest getTableNames) {
+ return new GetTableResult(getTableNames(getTableNames), true);
+ }
+ throw new UnsupportedOperationException(req + " not yet implemented");
+ }
+
+ private List<ExtendedTableInfo> getTablesExt(GetTablesExtRequest req) throws
MetaException, UnknownDBException {
+ List<ExtendedTableInfo> ret = new ArrayList<ExtendedTableInfo>();
+ String pattern = req.getTableNamePattern();
+ List<String> processorCapabilities = req.getProcessorCapabilities();
+ int limit = req.getLimit();
+ String catalog = req.isSetCatalog() ? req.getCatalog() :
getDefaultCatalog(conf);
+ String database = req.getDatabase();
+ String processorId = req.getProcessorIdentifier();
+ List<String> tables = ms.getTables(catalog, database, pattern, null,
limit);
+ LOG.debug("get_tables_ext:getTables() returned {}", tables.size());
+ tables = FilterUtils.filterTableNamesIfEnabled(filterHook != null,
filterHook,
+ catalog, database, tables);
+ if (tables.isEmpty()) {
+ return ret;
+ }
+ List<Table> tObjects = ms.getTableObjectsByName(catalog, database, tables);
+ LOG.debug("get_tables_ext:getTableObjectsByName() returned {}",
tObjects.size());
+ if (processorCapabilities == null || processorCapabilities.isEmpty() ||
+ processorCapabilities.contains("MANAGERAWMETADATA")) {
+ LOG.info("Skipping translation for processor with {}", processorId);
+ } else {
+ if (transformer != null) {
+ Map<Table, List<String>> retMap = transformer.transform(tObjects,
processorCapabilities, processorId);
+ for (Map.Entry<Table, List<String>> entry : retMap.entrySet()) {
+ LOG.debug("Table " + entry.getKey().getTableName() + " requires " +
Arrays.toString((entry.getValue()).toArray()));
+ ret.add(convertTableToExtendedTable(entry.getKey(),
entry.getValue(), req.getRequestedFields()));
+ }
+ } else {
+ for (Table table : tObjects) {
+ ret.add(convertTableToExtendedTable(table, processorCapabilities,
req.getRequestedFields()));
+ }
+ }
+ }
+ return ret;
+ }
+
+ private ExtendedTableInfo convertTableToExtendedTable(Table table,
+ List<String> processorCapabilities, int mask) {
+ ExtendedTableInfo extTable = new ExtendedTableInfo(table.getTableName());
+ if ((mask & GetTablesExtRequestFields.ACCESS_TYPE.getValue()) ==
GetTablesExtRequestFields.ACCESS_TYPE.getValue()) {
+ extTable.setAccessType(table.getAccessType());
+ }
+
+ if ((mask & GetTablesExtRequestFields.PROCESSOR_CAPABILITIES.getValue())
+ == GetTablesExtRequestFields.PROCESSOR_CAPABILITIES.getValue()) {
+
extTable.setRequiredReadCapabilities(table.getRequiredReadCapabilities());
+
extTable.setRequiredWriteCapabilities(table.getRequiredWriteCapabilities());
+ }
+
+ return extTable;
+ }
+
+ /**
+ * This function retrieves table from metastore. If getColumnStats flag is
true,
+ * then engine should be specified so the table is retrieve with the column
stats
+ * for that engine.
+ */
+ private Table getTable(GetTableRequest getTableRequest) throws
MetaException, NoSuchObjectException {
+
+ Preconditions.checkArgument(!getTableRequest.isGetColumnStats() ||
getTableRequest.getEngine() != null,
+ "To retrieve column statistics with a table, engine parameter cannot
be null");
+
+ if (isInTest) {
+ assertClientHasCapability(getTableRequest.getCapabilities(),
ClientCapability.TEST_CAPABILITY, "Hive tests",
+ "get_table_req");
+ }
+
+ Table t = getTableCore(getTableRequest);
+ if (MetaStoreUtils.isInsertOnlyTableParam(t.getParameters())) {
+ assertClientHasCapability(getTableRequest.getCapabilities(),
ClientCapability.INSERT_ONLY_TABLES,
+ "insert-only tables", "get_table_req");
+ }
+
+ if (CollectionUtils.isEmpty(getTableRequest.getProcessorCapabilities()) ||
getTableRequest
+ .getProcessorCapabilities().contains("MANAGERAWMETADATA")) {
+ LOG.info("Skipping translation for processor with " +
getTableRequest.getProcessorIdentifier());
+ } else {
+ if (transformer != null) {
+ List<Table> tList = new ArrayList<>();
+ tList.add(t);
+ Map<Table, List<String>> ret = transformer
+ .transform(tList, getTableRequest.getProcessorCapabilities(),
getTableRequest.getProcessorIdentifier());
+ if (ret.size() > 1) {
+ LOG.warn("Unexpected resultset size:{}", ret.size());
+ throw new MetaException("Unexpected result from metadata
transformer:return list size is " + ret.size());
+ }
+ t = ret.keySet().iterator().next();
+ }
+ }
+
+ ((HMSHandler) handler).firePreEvent(new PreReadTableEvent(t, handler));
+ return t;
+ }
+
+ /**
+ * This function retrieves table from metastore. If getColumnStats flag is
true,
+ * then engine should be specified so the table is retrieve with the column
stats
+ * for that engine.
+ */
+ private Table getTableCore(GetTableRequest getTableRequest) throws
MetaException, NoSuchObjectException {
+ Preconditions.checkArgument(!getTableRequest.isGetColumnStats() ||
getTableRequest.getEngine() != null,
+ "To retrieve column statistics with a table, engine parameter cannot
be null");
+ String catName = getTableRequest.getCatName();
+ String dbName = getTableRequest.getDbName();
+ String tblName = getTableRequest.getTblName();
+ Database db = null;
+ Table t;
+ try {
+ db = handler.get_database_core(catName, dbName);
+ } catch (Exception e) { /* appears exception is not thrown currently if db
doesnt exist */ }
+
+ if (MetaStoreUtils.isDatabaseRemote(db)) {
+ t =
DataConnectorProviderFactory.getDataConnectorProvider(db).getTable(tblName);
+ if (t == null) {
+ throw new NoSuchObjectException(TableName.getQualified(catName,
dbName, tblName) + " table not found");
+ }
+ t.setDbName(dbName);
+ return t;
+ }
+
+ t = ms.getTable(catName, dbName, tblName,
getTableRequest.getValidWriteIdList(), getTableRequest.getId());
+ if (t == null) {
+ throw new NoSuchObjectException(TableName.getQualified(catName, dbName,
tblName) + " table not found");
+ }
+
+ // If column statistics was requested and is valid fetch it.
+ if (getTableRequest.isGetColumnStats()) {
+ ColumnStatistics colStats = ms.getTableColumnStatistics(catName, dbName,
tblName,
+ StatsSetupConst.getColumnsHavingStats(t.getParameters()),
getTableRequest.getEngine(),
+ getTableRequest.getValidWriteIdList());
+ if (colStats != null) {
+ t.setColStats(colStats);
+ }
+ }
+ return t;
+ }
+
+ private List<Table> getTableObjects(GetTablesRequest req) throws TException {
+ String catName = req.isSetCatName() ? req.getCatName() :
getDefaultCatalog(conf);
+ Database database = handler.get_database_core(catName, req.getDbName());
+ if (isDatabaseRemote(database)) {
+ return getRemoteTableObjectsInternal(req.getDbName(), req.getTblNames(),
req.getTablesPattern());
+ }
+ return getTableObjectsInternal(req);
+ }
+
+ private List<Table> filterTablesByName(List<Table> tables, List<String>
tableNames) {
+ List<Table> filteredTables = new ArrayList<>();
+ for (Table table : tables) {
+ if (tableNames.contains(table.getTableName())) {
+ filteredTables.add(table);
+ }
+ }
+ return filteredTables;
+ }
+
+ private List<Table> getRemoteTableObjectsInternal(String dbname,
List<String> tableNames, String pattern) throws MetaException {
+ String[] parsedDbName = parseDbName(dbname, conf);
+ try {
+ // retrieve tables from remote database
+ Database db = handler.get_database_core(parsedDbName[CAT_NAME],
parsedDbName[DB_NAME]);
+ List<Table> tables =
DataConnectorProviderFactory.getDataConnectorProvider(db).getTables(null);
+
+ // filtered out undesired tables
+ if (tableNames != null) {
+ tables = filterTablesByName(tables, tableNames);
+ }
+
+ // set remote tables' local hive database reference
+ for (Table table : tables) {
+ table.setDbName(dbname);
+ }
+
+ return FilterUtils.filterTablesIfEnabled(filterHook != null, filterHook,
tables);
+ } catch (Exception e) {
+ LOG.warn("Unexpected exception while getting table(s) in remote database
" + dbname , e);
+ if (isInTest) {
+ // ignore the exception
+ return new ArrayList<Table>();
+ } else {
+ throw newMetaException(e);
+ }
+ }
+ }
+
+ private List<Table> getTableObjectsInternal(GetTablesRequest req)
+ throws MetaException, InvalidOperationException, UnknownDBException {
+ if (isInTest) {
+ assertClientHasCapability(req.getCapabilities(),
ClientCapability.TEST_CAPABILITY,
+ "Hive tests", "get_table_objects_by_name_req");
+ }
+
+ GetProjectionsSpec projectionsSpec = req.getProjectionSpec();
+ if (projectionsSpec != null) {
+ if (!projectionsSpec.isSetFieldList() &&
(projectionsSpec.isSetIncludeParamKeyPattern() ||
+ projectionsSpec.isSetExcludeParamKeyPattern())) {
+ throw new InvalidOperationException("Include and Exclude Param key are
not supported.");
+ }
+ }
+
+ String dbName = req.getDbName();
+ String catName = req.isSetCatName() ? req.getCatName() :
getDefaultCatalog(conf);
+ List<Table> tables = new ArrayList<>();
+ int tableBatchSize = MetastoreConf.getIntVar(conf,
MetastoreConf.ConfVars.BATCH_RETRIEVE_MAX);
+ if (dbName == null || dbName.isEmpty()) {
+ throw new UnknownDBException("DB name is null or empty");
+ }
+ List<String> tableNames = req.getTblNames();
+ if(req.getTablesPattern() != null) {
+ tables = ms.getTableObjectsByName(catName, dbName, tableNames,
projectionsSpec, req.getTablesPattern());
+ } else {
+ if (tableNames == null) {
+ throw new InvalidOperationException(dbName + " cannot find null
tables");
+ }
+
+ // The list of table names could contain duplicates.
RawStore.getTableObjectsByName()
+ // only guarantees returning no duplicate table objects in one batch. If
we need
+ // to break into multiple batches, remove duplicates first.
+ List<String> distinctTableNames = tableNames;
+ if (distinctTableNames.size() > tableBatchSize) {
+ List<String> lowercaseTableNames = new ArrayList<>();
+ for (String tableName : tableNames) {
+ lowercaseTableNames.add(normalizeIdentifier(tableName));
+ }
+ distinctTableNames = new ArrayList<>(new
HashSet<>(lowercaseTableNames));
+ }
+
+ int startIndex = 0;
+ // Retrieve the tables from the metastore in batches. Some databases like
+ // Oracle cannot have over 1000 expressions in a in-list
+ while (startIndex < distinctTableNames.size()) {
+ int endIndex = Math.min(startIndex + tableBatchSize,
distinctTableNames.size());
+ tables.addAll(ms.getTableObjectsByName(catName, dbName,
distinctTableNames.subList(
+ startIndex, endIndex), projectionsSpec, null));
+ startIndex = endIndex;
+ }
+ }
+ for (Table t : tables) {
+ if (t.getParameters() != null &&
MetaStoreUtils.isInsertOnlyTableParam(t.getParameters())) {
+ assertClientHasCapability(req.getCapabilities(),
ClientCapability.INSERT_ONLY_TABLES,
+ "insert-only tables", "get_table_req");
+ }
+ }
+
+ tables = FilterUtils.filterTablesIfEnabled(filterHook != null, filterHook,
tables);
+ return tables;
+ }
+
+ private void assertClientHasCapability(ClientCapabilities client,
+ ClientCapability value, String what, String call) throws MetaException {
+ if (!doesClientHaveCapability(client, value)) {
+ throw new MetaException("Your client does not appear to support " + what
+ ". To skip"
+ + " capability checks, please set " +
MetastoreConf.ConfVars.CAPABILITY_CHECK.toString()
+ + " to false. This setting can be set globally, or on the client for
the current"
+ + " metastore session. Note that this may lead to incorrect results,
data loss,"
+ + " undefined behavior, etc. if your client is actually
incompatible. You can also"
+ + " specify custom client capabilities via " + call + " API.");
+ }
+ }
+
+ private boolean doesClientHaveCapability(ClientCapabilities client,
ClientCapability value) {
+ if (!MetastoreConf.getBoolVar(conf,
MetastoreConf.ConfVars.CAPABILITY_CHECK)) {
+ return true;
+ }
+ return (client != null && client.isSetValues() &&
client.getValues().contains(value));
+ }
+
+ private List<String> getTableNames(GetTableNamesRequest getNamesReq) throws
TException {
+ String catName = getNamesReq.catName;
+ String dbname = getNamesReq.dbName;
+ try {
+ Database database = handler.get_database_core(catName, dbname);
+ if (isDatabaseRemote(database)) {
+ return
DataConnectorProviderFactory.getDataConnectorProvider(database).getTableNames();
+ }
+ } catch (Exception e) {
+ throw newMetaException(e);
+ }
+ List<String> names;
+ if (getNamesReq.filter != null) {
+ names = ms.listTableNamesByFilter(catName, dbname, getNamesReq.filter,
getNamesReq.limit);
+ } else if (getNamesReq.tableType != null) {
+ names = ms.getTables(catName, dbname, getNamesReq.pattern,
+ TableType.valueOf(getNamesReq.tableType), -1);
+ } else if (getNamesReq.pattern != null) {
+ names = ms.getTables(catName, dbname, getNamesReq.pattern);
+ } else {
+ names = ms.getAllTables(catName, dbname);
+ }
+ if (filterHook != null && !names.isEmpty()) {
+ String tables = String.join("|", names);
+ List<TableMeta> filteredTableMetas = ms.getTableMeta(catName, dbname,
tables, null);
+ if (filteredTableMetas == null || filteredTableMetas.isEmpty()) {
+ return new ArrayList<>();
+ }
+ Set<String> filtered =
filteredTableMetas.stream().map(TableMeta::getTableName).collect(Collectors.toSet());
+ List<String> result = names.stream().filter(filtered::contains).toList();
+ names = new ArrayList<>(result);
+ }
Review Comment:
When server-side filtering is enabled (filterHook != null), this code
fetches TableMeta via RawStore but never applies the filter hook (e.g.,
FilterUtils.filterTableMetasIfEnabled(...)). As a result, `filtered` will
contain all table names and no filtering actually occurs, potentially leaking
unauthorized table names via
get_tables/get_all_tables/get_tables_by_type/get_table_names_by_filter paths
that now route here. Apply the filter hook to the TableMeta list before
deriving the allowed table-name set (or use an existing FilterUtils helper that
correctly invokes the hook).
--
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]
