Skip to content

Remove redundant checks and misleading comments in Kudu #17010

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.

Sign up for GitHub

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

Merged
ebyhr merged 1 commit into from
Apr 14, 2023
+0 −38
Merged

Remove redundant checks and misleading comments in Kudu #17010

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 0 additions & 38 deletions plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduConnectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,8 @@
import java.util.regex.Pattern;

import static io.trino.plugin.kudu.KuduQueryRunnerFactory.createKuduQueryRunnerTpch;
import static io.trino.spi.connector.ConnectorMetadata.MODIFYING_ROWS_MESSAGE;
import static io.trino.spi.type.VarcharType.VARCHAR;
import static io.trino.testing.MaterializedResult.resultBuilder;
import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_CREATE_TABLE;
import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_DELETE;
import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_ROW_LEVEL_DELETE;
import static io.trino.testing.TestingNames.randomNameSuffix;
import static java.lang.String.format;
import static java.util.Locale.ENGLISH;
Expand Down Expand Up @@ -826,9 +822,6 @@ public void testLimitPushdown()
@Override
public void testDeleteWithComplexPredicate()
{
skipTestUnless(hasBehavior(SUPPORTS_DELETE));

// TODO (https://github.com/trinodb/trino/issues/5901) Use longer table name once Oracle version is updated
withTableName("test_delete_complex", tableName -> {
assertUpdate(createTableForWrites("CREATE TABLE %s %s".formatted(tableName, ORDER_COLUMNS)));
assertUpdate("INSERT INTO " + tableName + " SELECT * FROM orders", 15000);
Expand All @@ -847,9 +840,6 @@ public void testDeleteWithComplexPredicate()
public void testDeleteWithSubquery()
{
// TODO (https://github.com/trinodb/trino/issues/13210) Migrate these tests to AbstractTestEngineOnlyQueries
skipTestUnless(hasBehavior(SUPPORTS_DELETE));

// TODO (https://github.com/trinodb/trino/issues/5901) Use longer table name once Oracle version is updated
withTableName("test_delete_subquery", tableName -> {
assertUpdate(createTableForWrites("CREATE TABLE %s %s".formatted(tableName, NATION_COLUMNS)));
assertUpdate("INSERT INTO " + tableName + " SELECT * FROM nation", 25);
Expand All @@ -859,7 +849,6 @@ public void testDeleteWithSubquery()
"SELECT * FROM nation WHERE regionkey IN (SELECT regionkey FROM region WHERE name NOT LIKE 'A%')");
});

// TODO (https://github.com/trinodb/trino/issues/5901) Use longer table name once Oracle version is updated
withTableName("test_delete_subquery", tableName -> {
assertUpdate(createTableForWrites("CREATE TABLE %s %s".formatted(tableName, ORDER_COLUMNS)));
assertUpdate("INSERT INTO " + tableName + " SELECT * FROM orders", 15000);
Expand Down Expand Up @@ -897,9 +886,6 @@ public void testDeleteWithSubquery()
@Override
public void testDeleteWithSemiJoin()
{
skipTestUnless(hasBehavior(SUPPORTS_DELETE));

// TODO (https://github.com/trinodb/trino/issues/5901) Use longer table name once Oracle version is updated
withTableName("test_delete_semijoin", tableName -> {
assertUpdate(createTableForWrites("CREATE TABLE %s %s".formatted(tableName, NATION_COLUMNS)));
assertUpdate("INSERT INTO " + tableName + " SELECT * FROM nation", 25);
Expand All @@ -916,7 +902,6 @@ public void testDeleteWithSemiJoin()
" OR regionkey IN (SELECT regionkey FROM region WHERE length(comment) >= 50)");
});

// TODO (https://github.com/trinodb/trino/issues/5901) Use longer table name once Oracle version is updated
withTableName("test_delete_semijoin", tableName -> {
assertUpdate(createTableForWrites("CREATE TABLE %s %s".formatted(tableName, ORDER_COLUMNS)));
assertUpdate("INSERT INTO " + tableName + " SELECT * FROM orders", 15000);
Expand All @@ -938,8 +923,6 @@ public void testDeleteWithSemiJoin()
@Override
public void testDeleteWithVarcharPredicate()
{
skipTestUnless(hasBehavior(SUPPORTS_DELETE));

withTableName("test_delete_varchar", tableName -> {
assertUpdate(createTableForWrites("CREATE TABLE %s %s".formatted(tableName, ORDER_COLUMNS)));
assertUpdate("INSERT INTO " + tableName + " SELECT * FROM orders", 15000);
Expand All @@ -948,29 +931,10 @@ public void testDeleteWithVarcharPredicate()
});
}

@Test
@Override
public void verifySupportsDeleteDeclaration()
{
if (hasBehavior(SUPPORTS_DELETE)) {
// Covered by testDeleteAllDataFromTable
return;
}

skipTestUnless(hasBehavior(SUPPORTS_CREATE_TABLE));
withTableName("test_supports_delete", tableName -> {
assertUpdate(createTableForWrites("CREATE TABLE %s %s".formatted(tableName, REGION_COLUMNS)));
assertUpdate("INSERT INTO " + tableName + " SELECT * FROM region", 5);
assertQueryFails("DELETE FROM " + tableName, MODIFYING_ROWS_MESSAGE);
});
}

@Test
@Override
public void testDeleteAllDataFromTable()
{
skipTestUnless(hasBehavior(SUPPORTS_CREATE_TABLE) && hasBehavior(SUPPORTS_DELETE));

withTableName("test_delete_all_data", tableName -> {
assertUpdate(createTableForWrites("CREATE TABLE %s %s".formatted(tableName, REGION_COLUMNS)));
assertUpdate("INSERT INTO " + tableName + " SELECT * FROM region", 5);
Expand All @@ -985,8 +949,6 @@ public void testDeleteAllDataFromTable()
@Override
public void testRowLevelDelete()
{
skipTestUnless(hasBehavior(SUPPORTS_CREATE_TABLE) && hasBehavior(SUPPORTS_ROW_LEVEL_DELETE));
// TODO (https://github.com/trinodb/trino/issues/5901) Use longer table name once Oracle version is updated
withTableName("test_row_delete", tableName -> {
assertUpdate(createTableForWrites("CREATE TABLE %s %s".formatted(tableName, REGION_COLUMNS)));
assertUpdate("INSERT INTO " + tableName + " SELECT * FROM region", 5);
Expand Down