-
Notifications
You must be signed in to change notification settings - Fork 4k
[fix](fe) Fix SHOW CATALOGS WHERE filtering #68299
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,14 +20,43 @@ | |
| import org.apache.doris.catalog.Column; | ||
| import org.apache.doris.catalog.Env; | ||
| import org.apache.doris.catalog.ScalarType; | ||
| import org.apache.doris.common.FeConstants; | ||
| import org.apache.doris.nereids.NereidsPlanner; | ||
| import org.apache.doris.nereids.StatementContext; | ||
| import org.apache.doris.nereids.analyzer.UnboundInlineTable; | ||
| import org.apache.doris.nereids.analyzer.UnboundResultSink; | ||
| import org.apache.doris.nereids.glue.LogicalPlanAdapter; | ||
| import org.apache.doris.nereids.trees.expressions.Alias; | ||
| import org.apache.doris.nereids.trees.expressions.Expression; | ||
| import org.apache.doris.nereids.trees.expressions.NamedExpression; | ||
| import org.apache.doris.nereids.trees.expressions.literal.BigIntLiteral; | ||
| import org.apache.doris.nereids.trees.expressions.literal.NullLiteral; | ||
| import org.apache.doris.nereids.trees.expressions.literal.StringLiteral; | ||
| import org.apache.doris.nereids.trees.plans.LimitPhase; | ||
| import org.apache.doris.nereids.trees.plans.PlanType; | ||
| import org.apache.doris.nereids.trees.plans.logical.LogicalFilter; | ||
| import org.apache.doris.nereids.trees.plans.logical.LogicalLimit; | ||
| import org.apache.doris.nereids.trees.plans.logical.LogicalPlan; | ||
| import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor; | ||
| import org.apache.doris.nereids.types.BigIntType; | ||
| import org.apache.doris.nereids.types.StringType; | ||
| import org.apache.doris.qe.AutoCloseConnectContext; | ||
| import org.apache.doris.qe.ConnectContext; | ||
| import org.apache.doris.qe.OriginStatement; | ||
| import org.apache.doris.qe.ResultSet; | ||
| import org.apache.doris.qe.ShowResultSet; | ||
| import org.apache.doris.qe.ShowResultSetMetaData; | ||
| import org.apache.doris.qe.StmtExecutor; | ||
| import org.apache.doris.qe.VariableMgr; | ||
| import org.apache.doris.thrift.TUniqueId; | ||
|
|
||
| import com.google.common.collect.ImmutableList; | ||
| import com.google.common.collect.ImmutableSet; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
| import java.util.Optional; | ||
| import java.util.UUID; | ||
|
|
||
| /** | ||
| * Represents the command for show all catalog or desc the specific catalog. | ||
|
|
@@ -52,11 +81,13 @@ public class ShowCatalogCommand extends ShowCommand { | |
|
|
||
| private final String catalogName; | ||
| private final String pattern; | ||
| private final Expression whereClause; | ||
|
|
||
| public ShowCatalogCommand(String catalogName, String pattern) { | ||
| public ShowCatalogCommand(String catalogName, String pattern, Expression whereClause) { | ||
| super(PlanType.SHOW_CATALOG_COMMAND); | ||
| this.catalogName = catalogName; | ||
| this.pattern = pattern; | ||
| this.whereClause = whereClause; | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -65,9 +96,104 @@ public ShowResultSet doRun(ConnectContext ctx, StmtExecutor executor) throws Exc | |
| .showCatalogs(catalogName, pattern, ctx.getCurrentCatalog() != null | ||
| ? ctx.getCurrentCatalog().getName() : null); | ||
|
|
||
| if (whereClause == null) { | ||
| return new ShowResultSet(getMetaData(), rows); | ||
| } | ||
|
|
||
| rows = executeFilter(ctx, rows); | ||
| return new ShowResultSet(getMetaData(), rows); | ||
| } | ||
|
|
||
| private List<NamedExpression> toExpressions(List<String> row) { | ||
| return ImmutableList.of( | ||
| new Alias(new BigIntLiteral(Long.parseLong(row.get(0))), "CatalogId"), | ||
| stringAlias(row.get(1), "CatalogName"), | ||
| stringAlias(row.get(2), "Type"), | ||
| stringAlias(row.get(3), "IsCurrent"), | ||
| stringAlias(row.get(4), "CreateTime"), | ||
| stringAlias(row.get(5), "LastUpdateTime"), | ||
| stringAlias(row.get(6), "Comment"), | ||
| stringAlias(row.get(7), "ErrorMsg")); | ||
| } | ||
|
|
||
| private List<NamedExpression> nullRow() { | ||
| return ImmutableList.of( | ||
| new Alias(new NullLiteral(BigIntType.INSTANCE), "CatalogId"), | ||
| new Alias(new NullLiteral(StringType.INSTANCE), "CatalogName"), | ||
| new Alias(new NullLiteral(StringType.INSTANCE), "Type"), | ||
| new Alias(new NullLiteral(StringType.INSTANCE), "IsCurrent"), | ||
| new Alias(new NullLiteral(StringType.INSTANCE), "CreateTime"), | ||
| new Alias(new NullLiteral(StringType.INSTANCE), "LastUpdateTime"), | ||
| new Alias(new NullLiteral(StringType.INSTANCE), "Comment"), | ||
| new Alias(new NullLiteral(StringType.INSTANCE), "ErrorMsg")); | ||
| } | ||
|
|
||
| private NamedExpression stringAlias(String value, String name) { | ||
| Expression literal = value == null || FeConstants.null_string.equals(value) | ||
| ? new NullLiteral(StringType.INSTANCE) : new StringLiteral(value); | ||
| return new Alias(literal, name); | ||
| } | ||
|
|
||
| private List<List<String>> executeFilter(ConnectContext outerContext, List<List<String>> rows) throws Exception { | ||
| ConnectContext filterContext = buildFilterContext(outerContext); | ||
| try (AutoCloseConnectContext ignored = new AutoCloseConnectContext(filterContext)) { | ||
| // The SHOW predicate must not replace the statement, state, or query id being audited by the caller. | ||
| if (rows.isEmpty()) { | ||
| executeFilterPlan(filterContext, filterPlan(nullRow(), true)); | ||
| return rows; | ||
| } | ||
|
|
||
| // Multi-row VALUES plans require BE execution. Filtering one row at a time keeps this path in FE | ||
| // and preserves the CatalogMgr order instead of applying a different SQL string collation. | ||
| List<List<String>> filteredRows = new ArrayList<>(rows.size()); | ||
| for (List<String> row : rows) { | ||
| filteredRows.addAll(executeFilterPlan(filterContext, filterPlan(toExpressions(row), false))); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Install the per-row |
||
| } | ||
| return filteredRows; | ||
| } | ||
| } | ||
|
|
||
| private LogicalPlan filterPlan(List<NamedExpression> value, boolean empty) { | ||
| LogicalPlan input = new UnboundInlineTable(ImmutableList.of(value)); | ||
| if (empty) { | ||
| // Keep a typed zero-row relation so invalid WHERE expressions are still rejected. | ||
| input = new LogicalLimit<>(0, 0, LimitPhase.ORIGIN, input); | ||
| } | ||
| return new UnboundResultSink<>(new LogicalFilter<>(ImmutableSet.of(whereClause), input)); | ||
| } | ||
|
|
||
| private List<List<String>> executeFilterPlan(ConnectContext filterContext, LogicalPlan plan) throws Exception { | ||
| StatementContext statementContext = new StatementContext( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Preserve bound server-prepared parameters in this inner statement context. COM_STMT_EXECUTE has already placed the literal for |
||
| filterContext, new OriginStatement(toString(), 0)); | ||
| filterContext.setStatementContext(statementContext); | ||
| LogicalPlanAdapter adapter = new LogicalPlanAdapter(plan, statementContext); | ||
| NereidsPlanner planner = new NereidsPlanner(statementContext); | ||
| planner.plan(adapter, filterContext.getSessionVariable().toThrift()); | ||
| Optional<ResultSet> resultSet = planner.handleQueryInFe(adapter); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Keep these synthetic per-row plans out of the global SQL cache. With default |
||
| if (!resultSet.isPresent()) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Do not require every valid WHERE predicate to fold entirely in FE. For example, |
||
| throw new IllegalStateException("SHOW CATALOGS filter must be executable in FE"); | ||
| } | ||
| return resultSet.get().getResultRows(); | ||
| } | ||
|
|
||
| private ConnectContext buildFilterContext(ConnectContext outerContext) { | ||
| ConnectContext filterContext = new ConnectContext(); | ||
| filterContext.setSessionVariable(VariableMgr.cloneSessionVariable(outerContext.getSessionVariable())); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Preserve the caller's SQL-observable session state in the isolated evaluation context. This fresh context does not copy user variables, connection id, last query id, or the original statement clock: after |
||
| filterContext.setEnv(Env.getCurrentEnv()); | ||
| filterContext.changeDefaultCatalog(outerContext.getDefaultCatalog()); | ||
| filterContext.setDatabase(outerContext.getDatabase()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Preserve the existing dropped-current-catalog behavior when building this context. |
||
| filterContext.setCurrentUserIdentity(outerContext.getCurrentUserIdentity()); | ||
| filterContext.setAuthenticatedPrincipal(outerContext.getAuthenticatedPrincipal()); | ||
| filterContext.setAuthenticatedRoles(outerContext.getAuthenticatedRoles()); | ||
| filterContext.setRemoteIP(outerContext.getRemoteIP()); | ||
| filterContext.setNoAuth(outerContext.getNoAuth()); | ||
| filterContext.setIsTempUser(outerContext.getIsTempUser()); | ||
| UUID uuid = UUID.randomUUID(); | ||
| filterContext.setQueryId(new TUniqueId(uuid.getMostSignificantBits(), uuid.getLeastSignificantBits())); | ||
| filterContext.setStartTime(); | ||
| return filterContext; | ||
| } | ||
|
|
||
| @Override | ||
| public <R, C> R accept(PlanVisitor<R, C> visitor, C context) { | ||
| return visitor.visitShowCatalogCommand(this, context); | ||
|
|
@@ -89,6 +215,9 @@ public String toString() { | |
| sb.append("'"); | ||
| sb.append(pattern); | ||
| sb.append("'"); | ||
| } else if (whereClause != null) { | ||
| sb.append(" WHERE "); | ||
| sb.append(whereClause.toSql()); | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,143 @@ | ||
| // 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.doris.nereids.trees.plans.commands; | ||
|
|
||
| import org.apache.doris.catalog.Env; | ||
| import org.apache.doris.common.FeConstants; | ||
| import org.apache.doris.datasource.CatalogMgr; | ||
| import org.apache.doris.nereids.exceptions.AnalysisException; | ||
| import org.apache.doris.nereids.parser.NereidsParser; | ||
| import org.apache.doris.nereids.trees.plans.logical.LogicalPlan; | ||
| import org.apache.doris.qe.ShowResultSet; | ||
| import org.apache.doris.qe.StmtExecutor; | ||
| import org.apache.doris.thrift.TUniqueId; | ||
| import org.apache.doris.utframe.TestWithFeService; | ||
|
|
||
| import org.junit.jupiter.api.Assertions; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.mockito.MockedStatic; | ||
| import org.mockito.Mockito; | ||
|
|
||
| import java.util.Arrays; | ||
| import java.util.Collections; | ||
| import java.util.List; | ||
| import java.util.concurrent.atomic.AtomicInteger; | ||
|
|
||
| public class ShowCatalogCommandTest extends TestWithFeService { | ||
| @Test | ||
| void testWhereClauseIsNotTreatedAsLikePattern() { | ||
| LogicalPlan plan = new NereidsParser().parseSingle( | ||
| "SHOW CATALOGS WHERE CatalogName = 'internal'"); | ||
|
|
||
| Assertions.assertInstanceOf(ShowCatalogCommand.class, plan); | ||
| Assertions.assertTrue(plan.toString().startsWith("SHOW CATALOGS WHERE ")); | ||
| Assertions.assertFalse(plan.toString().contains(" LIKE ")); | ||
| } | ||
|
|
||
| @Test | ||
| void testWhereClauseFiltersCatalogRows() throws Exception { | ||
| String sql = "SHOW CATALOGS WHERE CatalogName = 'internal'"; | ||
| LogicalPlan plan = new NereidsParser().parseSingle(sql); | ||
| StmtExecutor executor = new StmtExecutor(connectContext, sql); | ||
|
|
||
| ShowResultSet resultSet = ((ShowCatalogCommand) plan).doRun(connectContext, executor); | ||
| Assertions.assertEquals(1, resultSet.getResultRows().size()); | ||
| Assertions.assertEquals("internal", resultSet.getResultRows().get(0).get(1)); | ||
|
|
||
| String nonMatchingSql = "SHOW CATALOGS WHERE CatalogName = 'missing'"; | ||
| LogicalPlan nonMatchingPlan = new NereidsParser().parseSingle(nonMatchingSql); | ||
| StmtExecutor nonMatchingExecutor = new StmtExecutor(connectContext, nonMatchingSql); | ||
| ShowResultSet emptyResultSet = ((ShowCatalogCommand) nonMatchingPlan) | ||
| .doRun(connectContext, nonMatchingExecutor); | ||
| Assertions.assertTrue(emptyResultSet.getResultRows().isEmpty()); | ||
| } | ||
|
|
||
| @Test | ||
| void testWhereClauseDoesNotMutateOuterExecutionState() throws Exception { | ||
| String sql = "SHOW CATALOGS WHERE CatalogName = 'internal'"; | ||
| ShowCatalogCommand command = (ShowCatalogCommand) new NereidsParser().parseSingle(sql); | ||
| StmtExecutor executor = new StmtExecutor(connectContext, sql); | ||
| TUniqueId queryId = new TUniqueId(123, 456); | ||
| connectContext.setQueryId(queryId); | ||
| connectContext.getState().setInternal(false); | ||
|
|
||
| command.doRun(connectContext, executor); | ||
|
|
||
| Assertions.assertEquals(queryId, connectContext.queryId()); | ||
| Assertions.assertFalse(connectContext.getState().isInternal()); | ||
| Assertions.assertNull(executor.getParsedStmt()); | ||
| } | ||
|
|
||
| @Test | ||
| void testWhereClauseIsAnalyzedForEmptyCatalogRows() throws Exception { | ||
| String sql = "SHOW CATALOGS WHERE MissingColumn = 'value'"; | ||
|
|
||
| Assertions.assertThrows(AnalysisException.class, | ||
| () -> executeWithCatalogRows(sql, Collections.emptyList())); | ||
| } | ||
|
|
||
| @Test | ||
| void testWhereClausePreservesCatalogManagerOrdering() throws Exception { | ||
| String supplementaryName = "catalog_" + Character.toString(0x10400); | ||
| String bmpName = "catalog_" + Character.toString(0xF900); | ||
| List<List<String>> catalogRows = List.of( | ||
| catalogRow("1", supplementaryName, "comment"), | ||
| catalogRow("2", bmpName, "comment")); | ||
| String sql = "SHOW CATALOGS WHERE CatalogName IN ('" + supplementaryName + "', '" + bmpName + "')"; | ||
|
|
||
| ShowResultSet resultSet = executeWithCatalogRows(sql, catalogRows); | ||
|
|
||
| Assertions.assertEquals(supplementaryName, resultSet.getResultRows().get(0).get(1)); | ||
| Assertions.assertEquals(bmpName, resultSet.getResultRows().get(1).get(1)); | ||
| } | ||
|
|
||
| @Test | ||
| void testWhereClauseUsesSqlNullSemantics() throws Exception { | ||
| String defaultTimeSql = "SHOW CATALOGS WHERE CreateTime IS NULL AND LastUpdateTime IS NULL"; | ||
| ShowResultSet defaultTimeResult = ((ShowCatalogCommand) new NereidsParser().parseSingle(defaultTimeSql)) | ||
| .doRun(connectContext, new StmtExecutor(connectContext, defaultTimeSql)); | ||
| Assertions.assertEquals("internal", defaultTimeResult.getResultRows().get(0).get(1)); | ||
|
|
||
| List<List<String>> restoredRows = List.of(catalogRow("2", "restored_catalog", null)); | ||
| ShowResultSet restoredResult = executeWithCatalogRows( | ||
| "SHOW CATALOGS WHERE Comment IS NULL", restoredRows); | ||
| Assertions.assertEquals("restored_catalog", restoredResult.getResultRows().get(0).get(1)); | ||
| } | ||
|
|
||
| private ShowResultSet executeWithCatalogRows(String sql, List<List<String>> catalogRows) throws Exception { | ||
| Env realEnv = Env.getCurrentEnv(); | ||
| Env mockedCurrentEnv = Mockito.mock(Env.class); | ||
| CatalogMgr catalogMgr = Mockito.mock(CatalogMgr.class); | ||
| Mockito.when(mockedCurrentEnv.getCatalogMgr()).thenReturn(catalogMgr); | ||
| Mockito.when(catalogMgr.showCatalogs(Mockito.nullable(String.class), Mockito.nullable(String.class), | ||
| Mockito.nullable(String.class))).thenReturn(catalogRows); | ||
| AtomicInteger calls = new AtomicInteger(); | ||
|
|
||
| try (MockedStatic<Env> mockedEnv = Mockito.mockStatic(Env.class, Mockito.CALLS_REAL_METHODS)) { | ||
| mockedEnv.when(Env::getCurrentEnv) | ||
| .thenAnswer(invocation -> calls.getAndIncrement() == 0 ? mockedCurrentEnv : realEnv); | ||
| ShowCatalogCommand command = (ShowCatalogCommand) new NereidsParser().parseSingle(sql); | ||
| return command.doRun(connectContext, new StmtExecutor(connectContext, sql)); | ||
| } | ||
| } | ||
|
|
||
| private List<String> catalogRow(String id, String name, String comment) { | ||
| return Arrays.asList(id, name, "test", "No", FeConstants.null_string, | ||
| FeConstants.null_string, comment, ""); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Avoid running a complete Nereids planning/translation cycle once per visible catalog. Each iteration creates a fresh planner and calls
plan, which reaches analysis, rewrites, memo optimization, post-processing, fragment splitting, and physical translation;CatalogMgr.showCatalogsplaces no bound on the number of external catalogs. This makes one metadata filter consume planner CPU and allocations proportional to catalog count. Please bind/plan once for the row set (carrying an ordinal if needed to retain manager order), or compile the predicate once for repeated row evaluation.