Skip to content

Commit

Permalink
Prevent DROP/RENAME TABLE IF EXISTS from silently ignoring views
Browse files Browse the repository at this point in the history
  • Loading branch information
@Riddle4045 @findepi
Riddle4045 authored and findepi committed Apr 28, 2022
1 parent 0a3b765 commit aeb75f2
Show file tree
Hide file tree
Showing 8 changed files with 83 additions and 92 deletions.
31 changes: 14 additions & 17 deletions core/trino-main/src/main/java/io/trino/execution/DropMaterializedViewTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,17 @@
import io.trino.execution.warnings.WarningCollector;
import io.trino.metadata.Metadata;
import io.trino.metadata.QualifiedObjectName;
import io.trino.metadata.TableHandle;
import io.trino.security.AccessControl;
import io.trino.sql.tree.DropMaterializedView;
import io.trino.sql.tree.Expression;

import javax.inject.Inject;

import java.util.List;
import java.util.Optional;

import static com.google.common.util.concurrent.Futures.immediateVoidFuture;
import static io.trino.metadata.MetadataUtil.createQualifiedObjectName;
import static io.trino.spi.StandardErrorCode.TABLE_NOT_FOUND;
import static io.trino.spi.StandardErrorCode.GENERIC_USER_ERROR;
import static io.trino.sql.analyzer.SemanticExceptions.semanticException;
import static java.util.Objects.requireNonNull;

Expand Down Expand Up @@ -64,21 +62,20 @@ public ListenableFuture<Void> execute(
QualifiedObjectName name = createQualifiedObjectName(session, statement, statement.getName());

if (!metadata.isMaterializedView(session, name)) {
if (metadata.isView(session, name)) {
throw semanticException(
GENERIC_USER_ERROR,
statement,
"Materialized view '%s' does not exist, but a view with that name exists. Did you mean DROP VIEW %s?", name, name);
}
if (metadata.getTableHandle(session, name).isPresent()) {
throw semanticException(
GENERIC_USER_ERROR,
statement,
"Materialized view '%s' does not exist, but a table with that name exists. Did you mean DROP TABLE %s?", name, name);
}
if (!statement.isExists()) {
if (metadata.isView(session, name)) {
throw semanticException(
TABLE_NOT_FOUND,
statement,
"Materialized view '%s' does not exist, but a view with that name exists. Did you mean DROP VIEW %s?", name, name);
}
Optional<TableHandle> table = metadata.getTableHandle(session, name);
if (table.isPresent()) {
throw semanticException(
TABLE_NOT_FOUND,
statement,
"Materialized view '%s' does not exist, but a table with that name exists. Did you mean DROP TABLE %s?", name, name);
}
throw semanticException(TABLE_NOT_FOUND, statement, "Materialized view '%s' does not exist", name);
throw semanticException(GENERIC_USER_ERROR, statement, "Materialized view '%s' does not exist", name);
}
return immediateVoidFuture();
}
Expand Down
23 changes: 9 additions & 14 deletions core/trino-main/src/main/java/io/trino/execution/DropTableTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

import static com.google.common.util.concurrent.Futures.immediateVoidFuture;
import static io.trino.metadata.MetadataUtil.createQualifiedObjectName;
import static io.trino.spi.StandardErrorCode.GENERIC_USER_ERROR;
import static io.trino.spi.StandardErrorCode.TABLE_NOT_FOUND;
import static io.trino.sql.analyzer.SemanticExceptions.semanticException;
import static java.util.Objects.requireNonNull;
Expand Down Expand Up @@ -62,23 +63,17 @@ public ListenableFuture<Void> execute(
Session session = stateMachine.getSession();
QualifiedObjectName originalTableName = createQualifiedObjectName(session, statement, statement.getTableName());
if (metadata.isMaterializedView(session, originalTableName)) {
if (!statement.isExists()) {
throw semanticException(
TABLE_NOT_FOUND,
statement,
"Table '%s' does not exist, but a materialized view with that name exists. Did you mean DROP MATERIALIZED VIEW %s?", originalTableName, originalTableName);
}
return immediateVoidFuture();
throw semanticException(
GENERIC_USER_ERROR,
statement,
"Table '%s' does not exist, but a materialized view with that name exists. Did you mean DROP MATERIALIZED VIEW %s?", originalTableName, originalTableName);
}

if (metadata.isView(session, originalTableName)) {
if (!statement.isExists()) {
throw semanticException(
TABLE_NOT_FOUND,
statement,
"Table '%s' does not exist, but a view with that name exists. Did you mean DROP VIEW %s?", originalTableName, originalTableName);
}
return immediateVoidFuture();
throw semanticException(
GENERIC_USER_ERROR,
statement,
"Table '%s' does not exist, but a view with that name exists. Did you mean DROP VIEW %s?", originalTableName, originalTableName);
}

RedirectionAwareTableHandle redirectionAwareTableHandle = metadata.getRedirectionAwareTableHandle(session, originalTableName);
Expand Down
30 changes: 12 additions & 18 deletions core/trino-main/src/main/java/io/trino/execution/DropViewTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,17 @@
import io.trino.execution.warnings.WarningCollector;
import io.trino.metadata.Metadata;
import io.trino.metadata.QualifiedObjectName;
import io.trino.metadata.TableHandle;
import io.trino.security.AccessControl;
import io.trino.sql.tree.DropView;
import io.trino.sql.tree.Expression;

import javax.inject.Inject;

import java.util.List;
import java.util.Optional;

import static com.google.common.util.concurrent.Futures.immediateVoidFuture;
import static io.trino.metadata.MetadataUtil.createQualifiedObjectName;
import static io.trino.spi.StandardErrorCode.TABLE_NOT_FOUND;
import static io.trino.spi.StandardErrorCode.GENERIC_USER_ERROR;
import static io.trino.sql.analyzer.SemanticExceptions.semanticException;
import static java.util.Objects.requireNonNull;

Expand Down Expand Up @@ -64,25 +62,21 @@ public ListenableFuture<Void> execute(
QualifiedObjectName name = createQualifiedObjectName(session, statement, statement.getName());

if (metadata.isMaterializedView(session, name)) {
if (!statement.isExists()) {
throw semanticException(
TABLE_NOT_FOUND,
statement,
"View '%s' does not exist, but a materialized view with that name exists. Did you mean DROP MATERIALIZED VIEW %s?", name, name);
}
return immediateVoidFuture();
throw semanticException(
GENERIC_USER_ERROR,
statement,
"View '%s' does not exist, but a materialized view with that name exists. Did you mean DROP MATERIALIZED VIEW %s?", name, name);
}

if (!metadata.isView(session, name)) {
if (metadata.getTableHandle(session, name).isPresent()) {
throw semanticException(
GENERIC_USER_ERROR,
statement,
"View '%s' does not exist, but a table with that name exists. Did you mean DROP TABLE %s?", name, name);
}
if (!statement.isExists()) {
Optional<TableHandle> table = metadata.getTableHandle(session, name);
if (table.isPresent()) {
throw semanticException(
TABLE_NOT_FOUND,
statement,
"View '%s' does not exist, but a table with that name exists. Did you mean DROP TABLE %s?", name, name);
}
throw semanticException(TABLE_NOT_FOUND, statement, "View '%s' does not exist", name);
throw semanticException(GENERIC_USER_ERROR, statement, "View '%s' does not exist", name);
}
return immediateVoidFuture();
}
Expand Down
23 changes: 9 additions & 14 deletions core/trino-main/src/main/java/io/trino/execution/RenameTableTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import static com.google.common.util.concurrent.Futures.immediateVoidFuture;
import static io.trino.metadata.MetadataUtil.createQualifiedObjectName;
import static io.trino.spi.StandardErrorCode.CATALOG_NOT_FOUND;
import static io.trino.spi.StandardErrorCode.GENERIC_USER_ERROR;
import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED;
import static io.trino.spi.StandardErrorCode.SYNTAX_ERROR;
import static io.trino.spi.StandardErrorCode.TABLE_ALREADY_EXISTS;
Expand Down Expand Up @@ -72,23 +73,17 @@ public ListenableFuture<Void> execute(
QualifiedObjectName tableName = createQualifiedObjectName(session, statement, statement.getSource());

if (metadata.isMaterializedView(session, tableName)) {
if (!statement.isExists()) {
throw semanticException(
TABLE_NOT_FOUND,
statement,
"Table '%s' does not exist, but a materialized view with that name exists. Did you mean ALTER MATERIALIZED VIEW %s RENAME TO ...?", tableName, tableName);
}
return immediateVoidFuture();
throw semanticException(
GENERIC_USER_ERROR,
statement,
"Table '%s' does not exist, but a materialized view with that name exists. Did you mean ALTER MATERIALIZED VIEW %s RENAME TO ...?", tableName, tableName);
}

if (metadata.isView(session, tableName)) {
if (!statement.isExists()) {
throw semanticException(
TABLE_NOT_FOUND,
statement,
"Table '%s' does not exist, but a view with that name exists. Did you mean ALTER VIEW %s RENAME TO ...?", tableName, tableName);
}
return immediateVoidFuture();
throw semanticException(
GENERIC_USER_ERROR,
statement,
"Table '%s' does not exist, but a view with that name exists. Did you mean ALTER VIEW %s RENAME TO ...?", tableName, tableName);
}

RedirectionAwareTableHandle redirectionAwareTableHandle = metadata.getRedirectionAwareTableHandle(session, tableName);
Expand Down
20 changes: 11 additions & 9 deletions core/trino-main/src/test/java/io/trino/execution/TestDropMaterializedViewTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import org.testng.annotations.Test;

import static io.airlift.concurrent.MoreFutures.getFutureValue;
import static io.trino.spi.StandardErrorCode.TABLE_NOT_FOUND;
import static io.trino.spi.StandardErrorCode.GENERIC_USER_ERROR;
import static io.trino.testing.assertions.TrinoExceptionAssert.assertTrinoExceptionThrownBy;
import static org.assertj.core.api.Assertions.assertThat;

Expand All @@ -48,7 +48,7 @@ public void testDropNotExistingMaterializedView()
QualifiedName viewName = qualifiedName("not_existing_materialized_view");

assertTrinoExceptionThrownBy(() -> getFutureValue(executeDropMaterializedView(viewName, false)))
.hasErrorCode(TABLE_NOT_FOUND)
.hasErrorCode(GENERIC_USER_ERROR)
.hasMessage("Materialized view '%s' does not exist", viewName);
}

Expand All @@ -67,8 +67,8 @@ public void testDropMaterializedViewOnTable()
QualifiedObjectName tableName = qualifiedObjectName("existing_table");
metadata.createTable(testSession, CATALOG_NAME, someTable(tableName), false);

assertTrinoExceptionThrownBy(() -> getFutureValue(executeDropMaterializedView(asQualifiedName(tableName), false)))
.hasErrorCode(TABLE_NOT_FOUND)
assertTrinoExceptionThrownBy(() -> getFutureValue(executeDropMaterializedView(asQualifiedName(tableName), true)))
.hasErrorCode(GENERIC_USER_ERROR)
.hasMessage("Materialized view '%s' does not exist, but a table with that name exists. Did you mean DROP TABLE %s?", tableName, tableName);
}

Expand All @@ -78,8 +78,9 @@ public void testDropMaterializedViewOnTableIfExists()
QualifiedObjectName tableName = qualifiedObjectName("existing_table");
metadata.createTable(testSession, CATALOG_NAME, someTable(tableName), false);

getFutureValue(executeDropMaterializedView(asQualifiedName(tableName), true));
// no exception
assertTrinoExceptionThrownBy(() -> getFutureValue(executeDropMaterializedView(asQualifiedName(tableName), true)))
.hasErrorCode(GENERIC_USER_ERROR)
.hasMessage("Materialized view '%s' does not exist, but a table with that name exists. Did you mean DROP TABLE %s?", tableName, tableName);
}

@Test
Expand All @@ -89,7 +90,7 @@ public void testDropMaterializedViewOnView()
metadata.createView(testSession, asQualifiedObjectName(viewName), someView(), false);

assertTrinoExceptionThrownBy(() -> getFutureValue(executeDropMaterializedView(viewName, false)))
.hasErrorCode(TABLE_NOT_FOUND)
.hasErrorCode(GENERIC_USER_ERROR)
.hasMessage("Materialized view '%s' does not exist, but a view with that name exists. Did you mean DROP VIEW %s?", viewName, viewName);
}

Expand All @@ -99,8 +100,9 @@ public void testDropMaterializedViewOnViewIfExists()
QualifiedName viewName = qualifiedName("existing_view");
metadata.createView(testSession, asQualifiedObjectName(viewName), someView(), false);

getFutureValue(executeDropMaterializedView(viewName, true));
// no exception
assertTrinoExceptionThrownBy(() -> getFutureValue(executeDropMaterializedView(viewName, false)))
.hasErrorCode(GENERIC_USER_ERROR)
.hasMessage("Materialized view '%s' does not exist, but a view with that name exists. Did you mean DROP VIEW %s?", viewName, viewName);
}

private ListenableFuture<Void> executeDropMaterializedView(QualifiedName viewName, boolean exists)
Expand Down
15 changes: 9 additions & 6 deletions core/trino-main/src/test/java/io/trino/execution/TestDropTableTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.testng.annotations.Test;

import static io.airlift.concurrent.MoreFutures.getFutureValue;
import static io.trino.spi.StandardErrorCode.GENERIC_USER_ERROR;
import static io.trino.spi.StandardErrorCode.TABLE_NOT_FOUND;
import static io.trino.testing.assertions.TrinoExceptionAssert.assertTrinoExceptionThrownBy;
import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -68,7 +69,7 @@ public void testDropTableOnView()
metadata.createView(testSession, asQualifiedObjectName(viewName), someView(), false);

assertTrinoExceptionThrownBy(() -> getFutureValue(executeDropTable(viewName, false)))
.hasErrorCode(TABLE_NOT_FOUND)
.hasErrorCode(GENERIC_USER_ERROR)
.hasMessage("Table '%s' does not exist, but a view with that name exists. Did you mean DROP VIEW %s?", viewName, viewName);
}

Expand All @@ -78,8 +79,9 @@ public void testDropTableOnViewIfExists()
QualifiedName viewName = qualifiedName("existing_view");
metadata.createView(testSession, asQualifiedObjectName(viewName), someView(), false);

getFutureValue(executeDropTable(viewName, true));
// no exception
assertTrinoExceptionThrownBy(() -> getFutureValue(executeDropTable(viewName, true)))
.hasErrorCode(GENERIC_USER_ERROR)
.hasMessage("Table '%s' does not exist, but a view with that name exists. Did you mean DROP VIEW %s?", viewName, viewName);
}

@Test
Expand All @@ -89,7 +91,7 @@ public void testDropTableOnMaterializedView()
metadata.createMaterializedView(testSession, asQualifiedObjectName(viewName), someMaterializedView(), false, false);

assertTrinoExceptionThrownBy(() -> getFutureValue(executeDropTable(viewName, false)))
.hasErrorCode(TABLE_NOT_FOUND)
.hasErrorCode(GENERIC_USER_ERROR)
.hasMessage("Table '%s' does not exist, but a materialized view with that name exists. Did you mean DROP MATERIALIZED VIEW %s?", viewName, viewName);
}

Expand All @@ -99,8 +101,9 @@ public void testDropTableOnMaterializedViewIfExists()
QualifiedName viewName = qualifiedName("existing_materialized_view");
metadata.createMaterializedView(testSession, asQualifiedObjectName(viewName), someMaterializedView(), false, false);

getFutureValue(executeDropTable(viewName, true));
// no exception
assertTrinoExceptionThrownBy(() -> getFutureValue(executeDropTable(viewName, true)))
.hasErrorCode(GENERIC_USER_ERROR)
.hasMessage("Table '%s' does not exist, but a materialized view with that name exists. Did you mean DROP MATERIALIZED VIEW %s?", viewName, viewName);
}

private ListenableFuture<Void> executeDropTable(QualifiedName tableName, boolean exists)
Expand Down
18 changes: 10 additions & 8 deletions core/trino-main/src/test/java/io/trino/execution/TestDropViewTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import org.testng.annotations.Test;

import static io.airlift.concurrent.MoreFutures.getFutureValue;
import static io.trino.spi.StandardErrorCode.TABLE_NOT_FOUND;
import static io.trino.spi.StandardErrorCode.GENERIC_USER_ERROR;
import static io.trino.testing.assertions.TrinoExceptionAssert.assertTrinoExceptionThrownBy;
import static org.assertj.core.api.Assertions.assertThat;

Expand All @@ -48,7 +48,7 @@ public void testDropNotExistingView()
QualifiedName viewName = qualifiedName("not_existing_view");

assertTrinoExceptionThrownBy(() -> getFutureValue(executeDropView(viewName, false)))
.hasErrorCode(TABLE_NOT_FOUND)
.hasErrorCode(GENERIC_USER_ERROR)
.hasMessage("View '%s' does not exist", viewName);
}

Expand All @@ -68,7 +68,7 @@ public void testDropViewOnTable()
metadata.createTable(testSession, CATALOG_NAME, someTable(tableName), false);

assertTrinoExceptionThrownBy(() -> getFutureValue(executeDropView(asQualifiedName(tableName), false)))
.hasErrorCode(TABLE_NOT_FOUND)
.hasErrorCode(GENERIC_USER_ERROR)
.hasMessage("View '%s' does not exist, but a table with that name exists. Did you mean DROP TABLE %s?", tableName, tableName);
}

Expand All @@ -78,8 +78,9 @@ public void testDropViewOnTableIfExists()
QualifiedObjectName tableName = qualifiedObjectName("existing_table");
metadata.createTable(testSession, CATALOG_NAME, someTable(tableName), false);

getFutureValue(executeDropView(asQualifiedName(tableName), true));
// no exception
assertTrinoExceptionThrownBy(() -> getFutureValue(executeDropView(asQualifiedName(tableName), true)))
.hasErrorCode(GENERIC_USER_ERROR)
.hasMessage("View '%s' does not exist, but a table with that name exists. Did you mean DROP TABLE %s?", tableName, tableName);
}

@Test
Expand All @@ -89,7 +90,7 @@ public void testDropViewOnMaterializedView()
metadata.createMaterializedView(testSession, QualifiedObjectName.valueOf(viewName.toString()), someMaterializedView(), false, false);

assertTrinoExceptionThrownBy(() -> getFutureValue(executeDropView(viewName, false)))
.hasErrorCode(TABLE_NOT_FOUND)
.hasErrorCode(GENERIC_USER_ERROR)
.hasMessage("View '%s' does not exist, but a materialized view with that name exists. Did you mean DROP MATERIALIZED VIEW %s?", viewName, viewName);
}

Expand All @@ -99,8 +100,9 @@ public void testDropViewOnMaterializedViewIfExists()
QualifiedName viewName = qualifiedName("existing_materialized_view");
metadata.createMaterializedView(testSession, QualifiedObjectName.valueOf(viewName.toString()), someMaterializedView(), false, false);

getFutureValue(executeDropView(viewName, true));
// no exception
assertTrinoExceptionThrownBy(() -> getFutureValue(executeDropView(viewName, true)))
.hasErrorCode(GENERIC_USER_ERROR)
.hasMessage("View '%s' does not exist, but a materialized view with that name exists. Did you mean DROP MATERIALIZED VIEW %s?", viewName, viewName);
}

private ListenableFuture<Void> executeDropView(QualifiedName viewName, boolean exists)
Expand Down
Loading

0 comments on commit aeb75f2

Please sign in to comment.