HIVE-29690: Fix StackOverflowError when calling get_table_objects_by_name_req API with both tablenames and pattern

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) {
+      // case : tableNames == null and pattern != null
+      tables = ms.getTableObjectsByName(catName, dbName, null, projectionsSpec, req.getTablesPattern());
     } else {
       if (tableNames == null) {
         throw new InvalidOperationException(dbName + " cannot find null tables");
@@ -354,13 +360,16 @@ private List<Table> getTableObjectsInternal(GetTablesRequest req)
         distinctTableNames = new ArrayList<>(new HashSet<>(lowercaseTableNames));
       }
 
+      // case : tableNames != null and pattern == null -> batched flow
+      // case : tableNames != null and pattern != null -> batched flow with pattern filter
+      String tablesPattern = req.getTablesPattern();
       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), projectionsSpec, tablesPattern));
         startIndex = endIndex;
       }
     }

standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetExists.java

@@ -19,6 +19,7 @@
 package org.apache.hadoop.hive.metastore.client;
 
 import org.apache.hadoop.hive.metastore.ColumnType;
+import org.apache.hadoop.hive.metastore.HiveMetaStoreClient;
 import org.apache.hadoop.hive.metastore.IMetaStoreClient;
 import org.apache.hadoop.hive.metastore.MetaStoreTestUtils;
 import org.apache.hadoop.hive.metastore.TableType;
@@ -40,6 +41,7 @@
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
 
+import java.lang.reflect.Field;
 import java.util.*;
 
 import static org.apache.hadoop.hive.metastore.Warehouse.DEFAULT_DATABASE_NAME;
@@ -412,6 +414,66 @@ public void testGetTableObjectsByName() throws Exception {
 
   }
 
+  /**
+   * HIVE-29690: Case 3 — tableNames != null and pattern != null.
+   * The batched flow must apply the pattern as an additional filter, so only tables
+   * that match both the name list and the pattern are returned.
+   *
+   * The test is designed to fail if the non-batched (pattern-only) code path is taken:
+   * an extra table "test_table_to_find_extra" is created that matches the pattern but is
+   * intentionally absent from the name list.  A non-batched path would ignore the name list
+   * and return all three pattern-matching tables; the batched path returns only the two
+   * that appear in both the name list and the pattern.
+   */
+  @Test
+  public void testGetTableObjectsByNameAndPattern() throws Exception {
+    // Create a table that matches the pattern but is NOT in the requested name list.
+    // If the code wrongly takes the non-batched (pattern-only) path it returns this table
+    // too, making tables.size() == 3 and failing the assertion below.
+    new TableBuilder()
+        .setDbName(DEFAULT_DATABASE)
+        .setTableName("test_table_to_find_extra")
+        .addCol("test_col", "int")
+        .create(client, metaStore.getConf());
+
+    // Access the underlying Thrift iface so we can set both tblNames and tablesPattern.
+    HiveMetaStoreClient hmsClient = (HiveMetaStoreClient) client;
+    Field field = ThriftHiveMetaStoreClient.class.getDeclaredField("client");
+    field.setAccessible(true);
+    ThriftHiveMetastore.Iface iface =
+        (ThriftHiveMetastore.Iface) field.get(hmsClient.getThriftClient());
+
+    // Name list contains test_table_to_find_1 and test_table_to_find_2, but NOT
+    // test_table_to_find_extra.  The pattern matches all three.
+    // Batched path  → intersection of name list ∩ pattern → 2 tables  (correct)
+    // Non-batched   → all tables matching pattern (name list ignored) → 3 tables (wrong)
+    List<String> requestedNames = Arrays.asList(
+        testTables[2].getTableName(),  // "test_table_to_find_1"
+        testTables[3].getTableName()   // "test_table_to_find_2"
+    );
+
+    GetTablesRequest req = new GetTablesRequest(DEFAULT_DATABASE);
+    req.setCatName(testTables[0].getCatName());
+    req.setTblNames(requestedNames);
+    req.setTablesPattern("test_table_to_find_*");
+    req.setCapabilities(ThriftHiveMetaStoreClient.TEST_VERSION);
+
+    List<Table> tables = iface.get_table_objects_by_name_req(req).getTables();
+    Assert.assertEquals(
+        "Batched path must return only the 2 tables present in both the name list and the pattern; "
+            + "3 results would mean the non-batched (pattern-only) path was taken",
+        2, tables.size());
+    Set<String> returnedNames = new HashSet<>();
+    for (Table t : tables) {
+      returnedNames.add(t.getTableName());
+    }
+    Assert.assertTrue(returnedNames.contains(testTables[2].getTableName()));
+    Assert.assertTrue(returnedNames.contains(testTables[3].getTableName()));
+    Assert.assertFalse(
+        "test_table_to_find_extra was not in the name list and must not be returned",
+        returnedNames.contains("test_table_to_find_extra"));
+  }
+
   @Test
   public void testGetTableObjectsWithProjectionOfSingleField() throws Exception {
     List<String> tableNames = new ArrayList<>();

standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/handler/TestGetTableHandlerBatching.java

@@ -0,0 +1,101 @@
+/*
+ * 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 org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.IHMSHandler;
+import org.apache.hadoop.hive.metastore.RawStore;
+import org.apache.hadoop.hive.metastore.api.Database;
+import org.apache.hadoop.hive.metastore.api.DatabaseType;
+import org.apache.hadoop.hive.metastore.api.GetTablesRequest;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.util.Arrays;
+import java.util.Collections;
+
+/**
+ * Unit tests for HIVE-29690: verifies that when both tblNames and tablesPattern are set,
+ * GetTableHandler takes the batched code path and correctly passes the pattern to each
+ * batch call, rather than the non-batched (pattern-only) path that ignores the name list.
+ */
+public class TestGetTableHandlerBatching {
+
+  /**
+   * Verifies the batched path for Case 3 (tableNames != null, pattern != null).
+   *
+   * Strategy: set BATCH_RETRIEVE_MAX=1 so that 2 table names require 2 separate
+   * calls to RawStore.getTableObjectsByName.  Then use Mockito.verify to assert
+   * that the method was called exactly twice, each time with:
+   *   - a non-null sublist of size 1  (proves the name list was respected, not ignored)
+   *   - the pattern argument          (proves the pattern was forwarded to every batch)
+   *
+   * If the code took the non-batched (pattern-only) path instead, the method would be
+   * called exactly once with tblNames=null, and verify(times(2)) would fail.
+   */
+  @Test
+  public void testTableNamesAndPatternTakesBatchedPath() throws Exception {
+    // --- configuration ---
+    Configuration conf = MetastoreConf.newMetastoreConf();
+    // One name per batch so that 2 names → 2 calls.
+    MetastoreConf.setLongVar(conf, MetastoreConf.ConfVars.BATCH_RETRIEVE_MAX, 1);
+    // Disable HIVE_IN_TEST so the capability check inside getTableObjectsInternal is skipped.
+    MetastoreConf.setBoolVar(conf, MetastoreConf.ConfVars.HIVE_IN_TEST, false);
+
+    // --- mocks ---
+    RawStore ms = Mockito.mock(RawStore.class);
+    Mockito.when(ms.getTableObjectsByName(
+            Mockito.anyString(), Mockito.anyString(),
+            Mockito.anyList(), Mockito.any(), Mockito.anyString()))
+        .thenReturn(Collections.emptyList());
+
+    Database localDb = new Database("testdb", null, null, null);
+    localDb.setType(DatabaseType.NATIVE);
+
+    IHMSHandler handler = Mockito.mock(IHMSHandler.class);
+    Mockito.when(handler.getConf()).thenReturn(conf);
+    Mockito.when(handler.getMS()).thenReturn(ms);
+    Mockito.when(handler.getMetadataTransformer()).thenReturn(null);
+    Mockito.when(handler.getMetaFilterHook()).thenReturn(null);
+    Mockito.when(handler.get_database_core(Mockito.anyString(), Mockito.anyString()))
+        .thenReturn(localDb);
+
+    // --- request: two names + a pattern ---
+    GetTablesRequest req = new GetTablesRequest("testdb");
+    req.setCatName("hive");
+    req.setTblNames(Arrays.asList("test_table_1", "test_table_2"));
+    req.setTablesPattern("test_*");
+
+    GetTableHandler.getTables(null, handler, req, null);
+
+    // --- verify ---
+    // With BATCH_RETRIEVE_MAX=1, the while-loop must fire once per name.
+    // Each call must receive a non-null sublist (size 1) and the pattern.
+    // A non-batched (pattern-only) call would use tblNames=null and fire only once —
+    // both of which would cause the verification below to fail.
+    Mockito.verify(ms, Mockito.times(2))
+        .getTableObjectsByName(
+            Mockito.anyString(),                                   // catName
+            Mockito.anyString(),                                   // dbName
+            Mockito.argThat(list -> list != null && list.size() == 1), // one name per batch
+            Mockito.any(),                                         // projectionSpec
+            Mockito.eq("test_*"));                                 // pattern forwarded to every batch
+  }
+}