vikramahuja1001 commented on code in PR #6573:
URL: https://github.com/apache/hive/pull/6573#discussion_r3615436565
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/GetTableHandler.java:
##########
@@ -335,8 +335,14 @@ private List<Table>
getTableObjectsInternal(GetTablesRequest req)
List<Table> tables = new ArrayList<>();
int tableBatchSize = MetastoreConf.getIntVar(conf,
MetastoreConf.ConfVars.BATCH_RETRIEVE_MAX);
List<String> tableNames = req.getTblNames();
- if(req.getTablesPattern() != null) {
- tables = ms.getTableObjectsByName(catName, dbName, tableNames,
projectionsSpec, req.getTablesPattern());
+ // case : tableNames is empty -> return empty list
+ if (tableNames != null && tableNames.isEmpty()) {
+ return tables;
+ }
+
+ if (req.getTablesPattern() != null && tableNames == null) {
Review Comment:
Thanks for reviewing and your feedback @deniskuzZ @saihemanth-cloudera
@dengzhhu653 and apologies for later reply on this.
You all are absolutely right that within hive, all the callers of this API
are disciplined and only either pass tblNames or tablesPattern, never
both(verified through extensive code review). So this combination is never
reached in the normal Hive Code path.
However, get_table_objects_by_name_req is a public Thrift API and HMS is
used as a standalone metastore by other engines as well, which are not bound by
Hive's internal calling conventions. We faced this issue in our prod machine
using Spark engine which uses older hive clients, this is how this bug was
surfaced.
Also, the original code's response to this combination is a
StackOverflowError, which is never a acceptable behaviour for a server-side API
regardless of if the combination was intended. The API should either:
1. Handle the input gracefully
2. Return an InvalidOperationException explaining the behaviour.
The patch takes option 1 to avoid the exception and handle it gracefully.
@dengzhhu653 , filtering names by pattern client-side before calling the
metastore is one valid approach, but it shifts the responsibility on to the
caller who in case of other engines might not be aware of this restriction.
Also since HMS has a pattern matching logic, handling it server side is more
defensive and consistent.
--
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]
