Copilot commented on code in PR #6311:
URL: https://github.com/apache/hive/pull/6311#discussion_r2785686338
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -3691,51 +3453,61 @@ public List<String> get_partition_names(final String
db_name, final String tbl_n
}
@Override
- public List<String> fetch_partition_names_req(final PartitionsRequest
partitionReq)
+ public List<String> fetch_partition_names_req(final PartitionsRequest req)
throws NoSuchObjectException, MetaException {
- String catName = partitionReq.getCatName();
- String dbName = partitionReq.getDbName();
- String tbl_name = partitionReq.getTblName();
- startTableFunction("fetch_partition_names_req", catName, dbName, tbl_name);
- fireReadTablePreEvent(catName, dbName, tbl_name);
- List<String> ret = null;
- Exception ex = null;
- try {
- authorizeTableForPartitionMetadata(catName, dbName, tbl_name);
- ret = getMS().listPartitionNames(catName, dbName, tbl_name,
partitionReq.getMaxParts());
- ret = FilterUtils.filterPartitionNamesIfEnabled(isServerFilterEnabled,
- filterHook, catName, dbName, tbl_name, ret);
- } catch (Exception e) {
- ex = e;
- throw newMetaException(e);
- } finally {
- endFunction("fetch_partition_names_req", ret != null, ex, tbl_name);
+ String catName = req.isSetCatName() ? req.getCatName() :
getDefaultCatalog(conf);
+ String dbName = req.getDbName(), tblName = req.getTblName();
+ TableName tableName = new TableName(catName, dbName, tblName);
+ try {
+ return GetPartitionsHandler.getPartitionNames(
+ t -> startTableFunction("fetch_partition_names_req", catName,
dbName, tblName),
+ rex -> endFunction("fetch_partition_names_req",
+ rex.getLeft() != null && rex.getLeft().success(),
rex.getRight(), tableName.toString()), this, tableName,
+ new
GetPartitionsArgs.GetPartitionsArgsBuilder().max(req.getMaxParts()).build()).result();
+ } catch (TException ex) {
+ if (ex instanceof NoSuchObjectException e) {
+ // Keep it here just because some tests in TestListPartitions assume
NoSuchObjectException
+ // if the input is invalid.
+ if (StringUtils.isBlank(dbName) ||
StringUtils.isBlank(tableName.getTable())) {
+ throw e;
+ }
+ return Collections.emptyList();
Review Comment:
fetch_partition_names_req() returns Collections.emptyList() for any
NoSuchObjectException when db/table names are non-blank. With the refactor,
NoSuchObjectException can also come from FilterUtils.checkDbAndTableFilters
(server-side filtering/authorization), so this branch can silently convert an
auth/filtered-out condition into an empty partition list, changing the previous
behavior (which surfaced an exception). Consider rethrowing when the
NoSuchObjectException originates from filtering/authorization (e.g., when
server filtering is enabled), and only returning empty for the specific "table
not found" case / legacy ObjectStore behavior.
```suggestion
// For legacy ObjectStore behavior, only return an empty list for
the specific "table not found"
// case. For other causes (e.g., server-side
filtering/authorization), rethrow so that callers
// still see the exception rather than an empty result.
String message = e.getMessage();
if (message != null && message.contains("table not found")) {
return Collections.emptyList();
}
throw e;
```
--
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]
