Skip to content

Commit dcc2771

Browse files
Baymineu70b3
andauthored
[improvement](catalog) avoid loading every table for SHOW TABLES on external catalogs (#66080)
### What problem does this PR solve? Issue Number: close #66079 Problem Summary: `SHOW TABLES` on an external catalog iterated `dbIf.getTables()`, which eagerly initializes every table via the meta cache (one `getTableNullable()` -> remote metadata load per table). For catalogs with many tables this turned a cheap name listing into N remote loads. This adds a fast path for the common case only -- a non-verbose `SHOW TABLES` on an external catalog -- that lists names via `dbIf.getTableNamesOrEmptyWithLock()` without initializing any table. The per-table `SHOW` privilege filter is preserved (it is name-based and needs no table load), so no table name is leaked to a user who lacks `SHOW` on it. Everything else keeps the original `getTables()` loop unchanged: - Internal-catalog `SHOW TABLES` / `SHOW FULL TABLES`. - External-catalog `SHOW FULL TABLES` (verbose): it still needs `getMysqlType()` and the storage-format columns, so it takes the original path and its 4-column output (`Table_type`, `Storage_format`, `Inverted_index_storage_format`) is unchanged. - `SHOW VIEWS` (needs `getEngine()` to filter views) and `SHOW STREAMS`. ### Release note None (performance optimization; `SHOW TABLES` output for accessible tables is unchanged, `SHOW FULL TABLES` / `SHOW VIEWS` are unaffected). ### Check List (For Author) - Test: - Manual test on a live cluster with an HMS catalog: `SHOW TABLES` and `SHOW TABLES LIKE` list names via the fast path; `SHOW FULL TABLES` returns the correct 4 columns (name, `Table_type`, `Storage_format`, `Inverted_index_storage_format`) with no column mismatch. - Ran existing `ShowTableCommandTest` (2 passed); FE build (`build.sh --fe`) + checkstyle green. - No new unit test: exercising the external-catalog branch needs an ExternalCatalog with a working `getTableNamesOrEmptyWithLock()`, which is not available in the FE unit-test harness; a portable regression `.out` needs the docker HMS test environment. - Behavior changed: No material change (a non-loadable external table's name is now listed by `SHOW TABLES` where the old path silently omitted it on load failure, which is arguably more correct). - Does this need documentation: No --------- Co-authored-by: lbs <lucian1412@outlook.com>
1 parent 62707fd commit dcc2771

2 files changed

Lines changed: 192 additions & 0 deletions

File tree

fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ShowTableCommand.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import org.apache.doris.common.ErrorReport;
3232
import org.apache.doris.common.PatternMatcher;
3333
import org.apache.doris.common.PatternMatcherWrapper;
34+
import org.apache.doris.datasource.InternalCatalog;
3435
import org.apache.doris.mysql.privilege.PrivPredicate;
3536
import org.apache.doris.nereids.trees.plans.PlanType;
3637
import org.apache.doris.nereids.trees.plans.commands.info.AliasInfo;
@@ -174,6 +175,26 @@ public ShowResultSet doRun(ConnectContext ctx, StmtExecutor executor) throws Exc
174175
Preconditions.checkArgument(table.get().getType().equals(TableIf.TableType.STREAM));
175176
rows.add(Lists.newArrayList(table.get().getName()));
176177
}
178+
} else if (!(dbIf.getCatalog() instanceof InternalCatalog)
179+
&& !isVerbose && type.equals(PlanType.SHOW_TABLES)) {
180+
// Non-verbose SHOW TABLES on an external catalog: list names directly
181+
// instead of dbIf.getTables(), which loads every table via the meta
182+
// cache (one remote metadata load per table). The per-table SHOW priv
183+
// filter below must be kept (name-based, needs no table load).
184+
// NOTE: must use getTableNamesWithLock(), NOT getTableNamesOrEmptyWithLock():
185+
// the latter swallows the case-insensitive name-conflict / meta_names_mapping
186+
// exception and returns an empty set, silently hiding conflicting table names.
187+
for (String tableName : dbIf.getTableNamesWithLock()) {
188+
if (matcher != null && !matcher.match(tableName)) {
189+
continue;
190+
}
191+
if (!Env.getCurrentEnv().getAccessManager()
192+
.checkTblPriv(ConnectContext.get(), catalog, dbIf.getFullName(), tableName,
193+
PrivPredicate.SHOW)) {
194+
continue;
195+
}
196+
rows.add(Lists.newArrayList(tableName));
197+
}
177198
} else {
178199
for (TableIf tbl : dbIf.getTables()) {
179200
if (type.equals(PlanType.SHOW_VIEWS) && (tbl.getEngine() == null

fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/ShowTableCommandTest.java

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,35 @@
1818
package org.apache.doris.nereids.trees.plans.commands;
1919

2020
import org.apache.doris.backup.CatalogMocker;
21+
import org.apache.doris.catalog.DatabaseIf;
22+
import org.apache.doris.catalog.Env;
23+
import org.apache.doris.catalog.TableIf;
2124
import org.apache.doris.common.AnalysisException;
25+
import org.apache.doris.datasource.CatalogIf;
26+
import org.apache.doris.datasource.CatalogMgr;
2227
import org.apache.doris.datasource.InternalCatalog;
28+
import org.apache.doris.mysql.privilege.AccessControllerManager;
29+
import org.apache.doris.mysql.privilege.PrivPredicate;
2330
import org.apache.doris.nereids.trees.plans.PlanType;
2431
import org.apache.doris.qe.ConnectContext;
32+
import org.apache.doris.qe.ShowResultSet;
33+
import org.apache.doris.qe.StmtExecutor;
2534
import org.apache.doris.utframe.TestWithFeService;
2635

36+
import com.google.common.collect.ImmutableSet;
37+
import com.google.common.collect.Lists;
2738
import org.junit.jupiter.api.Assertions;
2839
import org.junit.jupiter.api.Test;
40+
import org.mockito.MockedStatic;
41+
import org.mockito.Mockito;
2942

3043
import java.io.IOException;
44+
import java.util.List;
3145

3246
public class ShowTableCommandTest extends TestWithFeService {
47+
private static final String CATALOG_NAME = "hive_catalog";
48+
private static final String DB_NAME = "hive_db";
49+
3350
private ConnectContext ctx;
3451

3552
private void runBefore() throws IOException {
@@ -63,4 +80,158 @@ void testInvalidate() throws Exception {
6380
"", false, PlanType.SHOW_TABLES);
6481
Assertions.assertThrows(AnalysisException.class, () -> command2.validate(ctx));
6582
}
83+
84+
/**
85+
* Bundle of mocks needed to drive {@link ShowTableCommand#doRun}: a mocked {@link ConnectContext}
86+
* whose {@code getEnv().getCatalogMgr().getCatalogOrAnalysisException(...).getDbOrAnalysisException(...)}
87+
* chain resolves to {@code dbIf}, wired to the given catalog (which decides whether
88+
* {@code dbIf.getCatalog() instanceof InternalCatalog} holds).
89+
*/
90+
private static final class ShowTableCommandMocks {
91+
private final ConnectContext ctx = Mockito.mock(ConnectContext.class);
92+
private final StmtExecutor executor = Mockito.mock(StmtExecutor.class);
93+
private final Env env = Mockito.mock(Env.class);
94+
private final AccessControllerManager accessControllerManager = Mockito.mock(AccessControllerManager.class);
95+
@SuppressWarnings("unchecked")
96+
private final DatabaseIf<TableIf> dbIf = Mockito.mock(DatabaseIf.class);
97+
98+
@SuppressWarnings("unchecked")
99+
ShowTableCommandMocks(CatalogIf<?> catalogIf) throws AnalysisException {
100+
CatalogMgr catalogMgr = Mockito.mock(CatalogMgr.class);
101+
Mockito.when(ctx.getEnv()).thenReturn(env);
102+
Mockito.when(env.getCatalogMgr()).thenReturn(catalogMgr);
103+
Mockito.when(env.getAccessManager()).thenReturn(accessControllerManager);
104+
Mockito.when(catalogMgr.getCatalogOrAnalysisException(Mockito.anyString())).thenReturn(catalogIf);
105+
Mockito.when(catalogIf.getDbOrAnalysisException(Mockito.anyString())).thenReturn(dbIf);
106+
Mockito.when(dbIf.getCatalog()).thenReturn(catalogIf);
107+
Mockito.when(dbIf.getFullName()).thenReturn(DB_NAME);
108+
}
109+
}
110+
111+
private static ShowResultSet runDoRun(ShowTableCommandMocks mocks, ShowTableCommand command) throws Exception {
112+
try (MockedStatic<Env> mockedEnv = Mockito.mockStatic(Env.class);
113+
MockedStatic<ConnectContext> mockedConnectContext = Mockito.mockStatic(ConnectContext.class)) {
114+
mockedEnv.when(Env::getCurrentEnv).thenReturn(mocks.env);
115+
// Lower-case-table-names off, so LIKE patterns are matched case-sensitively.
116+
mockedEnv.when(() -> Env.getLowerCaseTableNames(Mockito.anyString())).thenReturn(0);
117+
mockedConnectContext.when(ConnectContext::get).thenReturn(mocks.ctx);
118+
return command.doRun(mocks.ctx, mocks.executor);
119+
}
120+
}
121+
122+
@Test
123+
public void testExternalCatalogNonVerboseShowTablesUsesFastPath() throws Exception {
124+
CatalogIf<?> catalogIf = Mockito.mock(CatalogIf.class);
125+
ShowTableCommandMocks mocks = new ShowTableCommandMocks(catalogIf);
126+
Mockito.when(mocks.dbIf.getTableNamesWithLock())
127+
.thenReturn(ImmutableSet.of("t2", "t1", "t_filtered_out"));
128+
// Every table is visible except "t_filtered_out", which SHOW privilege denies.
129+
Mockito.when(mocks.accessControllerManager.checkTblPriv(
130+
Mockito.eq(mocks.ctx), Mockito.eq(CATALOG_NAME), Mockito.eq(DB_NAME),
131+
Mockito.anyString(), Mockito.eq(PrivPredicate.SHOW)))
132+
.thenAnswer(invocation -> !"t_filtered_out".equals(invocation.getArgument(3)));
133+
134+
ShowTableCommand command = new ShowTableCommand(DB_NAME, CATALOG_NAME, false, PlanType.SHOW_TABLES);
135+
ShowResultSet result = runDoRun(mocks, command);
136+
137+
// Behavior: names come back sorted, and the privilege-denied table is excluded.
138+
List<List<String>> rows = result.getResultRows();
139+
Assertions.assertEquals(2, rows.size());
140+
Assertions.assertEquals(Lists.newArrayList("t1"), rows.get(0));
141+
Assertions.assertEquals(Lists.newArrayList("t2"), rows.get(1));
142+
143+
// Call counts: the fast path must list names, and must never load every table.
144+
// It uses getTableNamesWithLock() (not the exception-swallowing ...OrEmpty... variant)
145+
// so a name-conflict exception raised while listing is still propagated.
146+
Mockito.verify(mocks.dbIf, Mockito.times(1)).getTableNamesWithLock();
147+
Mockito.verify(mocks.dbIf, Mockito.times(0)).getTables();
148+
}
149+
150+
@Test
151+
public void testExternalCatalogShowTablesLikePatternUsesFastPath() throws Exception {
152+
CatalogIf<?> catalogIf = Mockito.mock(CatalogIf.class);
153+
ShowTableCommandMocks mocks = new ShowTableCommandMocks(catalogIf);
154+
Mockito.when(mocks.dbIf.getTableNamesWithLock())
155+
.thenReturn(ImmutableSet.of("tbl1", "tbl2", "other_tbl"));
156+
Mockito.when(mocks.accessControllerManager.checkTblPriv(
157+
Mockito.eq(mocks.ctx), Mockito.eq(CATALOG_NAME), Mockito.eq(DB_NAME),
158+
Mockito.anyString(), Mockito.eq(PrivPredicate.SHOW)))
159+
.thenReturn(true);
160+
161+
// The mysql LIKE pattern is applied on the names-only fast path: "tbl_"
162+
// matches tbl1/tbl2, but not other_tbl.
163+
ShowTableCommand command = new ShowTableCommand(DB_NAME, CATALOG_NAME, false, "tbl_", null,
164+
PlanType.SHOW_TABLES);
165+
ShowResultSet result = runDoRun(mocks, command);
166+
167+
List<List<String>> rows = result.getResultRows();
168+
Assertions.assertEquals(2, rows.size());
169+
Assertions.assertEquals(Lists.newArrayList("tbl1"), rows.get(0));
170+
Assertions.assertEquals(Lists.newArrayList("tbl2"), rows.get(1));
171+
Mockito.verify(mocks.dbIf, Mockito.times(1)).getTableNamesWithLock();
172+
Mockito.verify(mocks.dbIf, Mockito.times(0)).getTables();
173+
}
174+
175+
@Test
176+
public void testExternalCatalogShowTablesPropagatesNameListingFailure() throws Exception {
177+
CatalogIf<?> catalogIf = Mockito.mock(CatalogIf.class);
178+
ShowTableCommandMocks mocks = new ShowTableCommandMocks(catalogIf);
179+
// Guards the getTableNamesWithLock() contract: a case-insensitive name-conflict
180+
// failure must surface as an error, not be swallowed into an empty result.
181+
Mockito.when(mocks.dbIf.getTableNamesWithLock()).thenThrow(
182+
new RuntimeException("Found conflicting table names under case-insensitive conditions"));
183+
184+
ShowTableCommand command = new ShowTableCommand(DB_NAME, CATALOG_NAME, false, PlanType.SHOW_TABLES);
185+
Assertions.assertThrows(RuntimeException.class, () -> runDoRun(mocks, command));
186+
Mockito.verify(mocks.dbIf, Mockito.times(0)).getTables();
187+
}
188+
189+
@Test
190+
public void testExternalCatalogVerboseShowTablesUsesSlowPath() throws Exception {
191+
CatalogIf<?> catalogIf = Mockito.mock(CatalogIf.class);
192+
ShowTableCommandMocks mocks = new ShowTableCommandMocks(catalogIf);
193+
Mockito.when(mocks.dbIf.getTables()).thenReturn(Lists.newArrayList());
194+
195+
// Verbose SHOW TABLES needs per-table metadata (storage format, etc.), so even on an
196+
// external catalog it must fall back to the slow path that loads every table.
197+
ShowTableCommand command = new ShowTableCommand(DB_NAME, CATALOG_NAME, true, PlanType.SHOW_TABLES);
198+
ShowResultSet result = runDoRun(mocks, command);
199+
200+
Assertions.assertTrue(result.getResultRows().isEmpty());
201+
Mockito.verify(mocks.dbIf, Mockito.times(1)).getTables();
202+
Mockito.verify(mocks.dbIf, Mockito.times(0)).getTableNamesWithLock();
203+
}
204+
205+
@Test
206+
public void testExternalCatalogShowViewsUsesSlowPath() throws Exception {
207+
CatalogIf<?> catalogIf = Mockito.mock(CatalogIf.class);
208+
ShowTableCommandMocks mocks = new ShowTableCommandMocks(catalogIf);
209+
Mockito.when(mocks.dbIf.getTables()).thenReturn(Lists.newArrayList());
210+
211+
// SHOW VIEWS needs the engine type of every table to filter views, so the name-only
212+
// fast path (guarded by PlanType.SHOW_TABLES) must not be taken here.
213+
ShowTableCommand command = new ShowTableCommand(DB_NAME, CATALOG_NAME, false, PlanType.SHOW_VIEWS);
214+
ShowResultSet result = runDoRun(mocks, command);
215+
216+
Assertions.assertTrue(result.getResultRows().isEmpty());
217+
Mockito.verify(mocks.dbIf, Mockito.times(1)).getTables();
218+
Mockito.verify(mocks.dbIf, Mockito.times(0)).getTableNamesWithLock();
219+
}
220+
221+
@Test
222+
public void testInternalCatalogNonVerboseShowTablesUsesSlowPath() throws Exception {
223+
InternalCatalog internalCatalog = Mockito.mock(InternalCatalog.class);
224+
ShowTableCommandMocks mocks = new ShowTableCommandMocks(internalCatalog);
225+
Mockito.when(mocks.dbIf.getTables()).thenReturn(Lists.newArrayList());
226+
227+
// The fast path is only for external catalogs; internal catalogs always take the
228+
// slow path regardless of verbosity.
229+
ShowTableCommand command = new ShowTableCommand(DB_NAME, InternalCatalog.INTERNAL_CATALOG_NAME, false,
230+
PlanType.SHOW_TABLES);
231+
ShowResultSet result = runDoRun(mocks, command);
232+
233+
Assertions.assertTrue(result.getResultRows().isEmpty());
234+
Mockito.verify(mocks.dbIf, Mockito.times(1)).getTables();
235+
Mockito.verify(mocks.dbIf, Mockito.times(0)).getTableNamesWithLock();
236+
}
66237
}

0 commit comments

Comments
 (0)