This is an automated email from the ASF dual-hosted git repository. jhyde pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 66c0d13a6b2fd4c04f049aace71a64d3d378b0dc Author: devozerov <[email protected]> AuthorDate: Sun Mar 14 11:02:59 2021 +0300 [CALCITE-4535] ServerDdlExecutor cannot execute DROP commands with qualified object names (Vladimir Ozerov) Close apache/calcite#2371 --- .../apache/calcite/server/ServerDdlExecutor.java | 73 ++++++++++------------ .../java/org/apache/calcite/test/ServerTest.java | 23 +++++++ 2 files changed, 55 insertions(+), 41 deletions(-) diff --git a/server/src/main/java/org/apache/calcite/server/ServerDdlExecutor.java b/server/src/main/java/org/apache/calcite/server/ServerDdlExecutor.java index fd636d9..51c5b5f 100644 --- a/server/src/main/java/org/apache/calcite/server/ServerDdlExecutor.java +++ b/server/src/main/java/org/apache/calcite/server/ServerDdlExecutor.java @@ -47,6 +47,7 @@ import org.apache.calcite.schema.impl.ViewTableMacro; import org.apache.calcite.sql.SqlCall; import org.apache.calcite.sql.SqlDataTypeSpec; import org.apache.calcite.sql.SqlIdentifier; +import org.apache.calcite.sql.SqlKind; import org.apache.calcite.sql.SqlLiteral; import org.apache.calcite.sql.SqlNode; import org.apache.calcite.sql.SqlNodeList; @@ -62,7 +63,6 @@ import org.apache.calcite.sql.ddl.SqlCreateSchema; import org.apache.calcite.sql.ddl.SqlCreateTable; import org.apache.calcite.sql.ddl.SqlCreateType; import org.apache.calcite.sql.ddl.SqlCreateView; -import org.apache.calcite.sql.ddl.SqlDropMaterializedView; import org.apache.calcite.sql.ddl.SqlDropObject; import org.apache.calcite.sql.ddl.SqlDropSchema; import org.apache.calcite.sql.dialect.CalciteSqlDialect; @@ -281,41 +281,54 @@ public class ServerDdlExecutor extends DdlExecutorImpl { * {@code DROP VIEW} commands. */ public void execute(SqlDropObject drop, CalcitePrepare.Context context) { - final List<String> path = context.getDefaultSchemaPath(); - CalciteSchema schema = context.getRootSchema(); - for (String p : path) { - schema = schema.getSubSchema(p, true); - } - final boolean existed; + final Pair<CalciteSchema, String> pair = schema(context, false, drop.name); + CalciteSchema schema = pair.left; + String objectName = pair.right; + assert objectName != null; + + boolean schemaExists = schema != null; + + boolean existed; switch (drop.getKind()) { case DROP_TABLE: case DROP_MATERIALIZED_VIEW: - existed = schema.removeTable(drop.name.getSimple()); - if (!existed && !drop.ifExists) { + Table materializedView = schemaExists && drop.getKind() == SqlKind.DROP_MATERIALIZED_VIEW + ? schema.plus().getTable(objectName) : null; + + existed = schemaExists && schema.removeTable(objectName); + if (existed) { + if (materializedView instanceof Wrapper) { + ((Wrapper) materializedView).maybeUnwrap(MaterializationKey.class) + .ifPresent(materializationKey -> { + MaterializationService.instance() + .removeMaterialization(materializationKey); + }); + } + } else if (!drop.ifExists) { throw SqlUtil.newContextException(drop.name.getParserPosition(), - RESOURCE.tableNotFound(drop.name.getSimple())); + RESOURCE.tableNotFound(objectName)); } break; case DROP_VIEW: // Not quite right: removes any other functions with the same name - existed = schema.removeFunction(drop.name.getSimple()); + existed = schemaExists && schema.removeFunction(objectName); if (!existed && !drop.ifExists) { throw SqlUtil.newContextException(drop.name.getParserPosition(), - RESOURCE.viewNotFound(drop.name.getSimple())); + RESOURCE.viewNotFound(objectName)); } break; case DROP_TYPE: - existed = schema.removeType(drop.name.getSimple()); + existed = schemaExists && schema.removeType(objectName); if (!existed && !drop.ifExists) { throw SqlUtil.newContextException(drop.name.getParserPosition(), - RESOURCE.typeNotFound(drop.name.getSimple())); + RESOURCE.typeNotFound(objectName)); } break; case DROP_FUNCTION: - existed = schema.removeFunction(drop.name.getSimple()); + existed = schemaExists && schema.removeFunction(objectName); if (!existed && !drop.ifExists) { throw SqlUtil.newContextException(drop.name.getParserPosition(), - RESOURCE.functionNotFound(drop.name.getSimple())); + RESOURCE.functionNotFound(objectName)); } break; case OTHER_DDL: @@ -356,24 +369,6 @@ public class ServerDdlExecutor extends DdlExecutorImpl { sql, schemaPath, pair.right, true, true); } - /** Executes a {@code DROP MATERIALIZED VIEW} command. */ - public void execute(SqlDropMaterializedView drop, - CalcitePrepare.Context context) { - final Pair<CalciteSchema, String> pair = schema(context, true, drop.name); - final Table table = pair.left.plus().getTable(pair.right); - if (table != null) { - // Materialized view exists. - execute((SqlDropObject) drop, context); - if (table instanceof Wrapper) { - ((Wrapper) table).maybeUnwrap(MaterializationKey.class) - .ifPresent(materializationKey -> { - MaterializationService.instance() - .removeMaterialization(materializationKey); - }); - } - } - } - /** Executes a {@code CREATE SCHEMA} command. */ public void execute(SqlCreateSchema create, CalcitePrepare.Context context) { @@ -395,15 +390,11 @@ public class ServerDdlExecutor extends DdlExecutorImpl { /** Executes a {@code DROP SCHEMA} command. */ public void execute(SqlDropSchema drop, CalcitePrepare.Context context) { - final List<String> path = context.getDefaultSchemaPath(); - CalciteSchema schema = context.getRootSchema(); - for (String p : path) { - schema = schema.getSubSchema(p, true); - } - final boolean existed = schema.removeSubSchema(drop.name.getSimple()); + final Pair<CalciteSchema, String> pair = schema(context, false, drop.name); + final boolean existed = pair.left != null && pair.left.removeSubSchema(pair.right); if (!existed && !drop.ifExists) { throw SqlUtil.newContextException(drop.name.getParserPosition(), - RESOURCE.schemaNotFound(drop.name.getSimple())); + RESOURCE.schemaNotFound(pair.right)); } } diff --git a/server/src/test/java/org/apache/calcite/test/ServerTest.java b/server/src/test/java/org/apache/calcite/test/ServerTest.java index 559e949..43fa8c9 100644 --- a/server/src/test/java/org/apache/calcite/test/ServerTest.java +++ b/server/src/test/java/org/apache/calcite/test/ServerTest.java @@ -57,6 +57,7 @@ import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -532,4 +533,26 @@ class ServerTest { } } } + + @Test public void testDropWithFullyQualifiedNameWhenSchemaDoesntExist() throws Exception { + try (Connection c = connect(); + Statement s = c.createStatement()) { + checkDropWithFullyQualifiedNameWhenSchemaDoesntExist(s, "schema", "Schema"); + checkDropWithFullyQualifiedNameWhenSchemaDoesntExist(s, "table", "Table"); + checkDropWithFullyQualifiedNameWhenSchemaDoesntExist(s, "materialized view", "Table"); + checkDropWithFullyQualifiedNameWhenSchemaDoesntExist(s, "view", "View"); + checkDropWithFullyQualifiedNameWhenSchemaDoesntExist(s, "type", "Type"); + checkDropWithFullyQualifiedNameWhenSchemaDoesntExist(s, "function", "Function"); + } + } + + private void checkDropWithFullyQualifiedNameWhenSchemaDoesntExist( + Statement statement, String objectType, String objectTypeInErrorMessage) throws Exception { + SQLException e = assertThrows(SQLException.class, () -> + statement.execute("drop " + objectType + " s.o"), + "expected error because the object doesn't exist"); + assertThat(e.getMessage(), containsString(objectTypeInErrorMessage + " 'O' not found")); + + statement.execute("drop " + objectType + " if exists s.o"); + } }
