This is an automated email from the ASF dual-hosted git repository.
dengzhhu653 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hive.git
The following commit(s) were added to refs/heads/master by this push:
new 9fc3480c2a3 HIVE-29690: Fix StackOverflowError when calling
get_table_objects_by_name_req API with both tablenames and pattern (#6573)
9fc3480c2a3 is described below
commit 9fc3480c2a3154d93b0f0b908f4a0d9d55811702
Author: Vikram Ahuja <[email protected]>
AuthorDate: Fri Jul 31 05:47:22 2026 +0530
HIVE-29690: Fix StackOverflowError when calling
get_table_objects_by_name_req API with both tablenames and pattern (#6573)
---
.../hive/metastore/handler/GetTableHandler.java | 15 ++-
.../hive/metastore/client/TestTablesGetExists.java | 62 +++++++++++++
.../handler/TestGetTableHandlerBatching.java | 101 +++++++++++++++++++++
3 files changed, 175 insertions(+), 3 deletions(-)
diff --git
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/GetTableHandler.java
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/GetTableHandler.java
index 6c69c693fd4..87d26cd5716 100644
---
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/GetTableHandler.java
+++
b/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;
}
}
diff --git
a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetExists.java
b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetExists.java
index cd51865da88..72e47773a19 100644
---
a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetExists.java
+++
b/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<>();
diff --git
a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/handler/TestGetTableHandlerBatching.java
b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/handler/TestGetTableHandlerBatching.java
new file mode 100644
index 00000000000..6a7920b0f8f
--- /dev/null
+++
b/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
+ }
+}
