From 20b6e854fc6dfe829f44a57111076c519e830b59 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Thu, 21 Apr 2022 15:31:44 +0200 Subject: [PATCH 1/2] Remove redundant skipTestUnlessSupportsDeletes method - test skipping can be simply done with `skipTestUnless(hasBehavior(SUPPORTS_DELETE));` - the correctness of SUPPORTS_DELETE declaration is already verified in `verifySupportsDeleteDeclaration` and doesn't need to be re-verified in other tests - a `skip...` method should just do a skip, not assertions, anyway --- .../kudu/AbstractKuduConnectorTest.java | 9 ------- .../io/trino/testing/BaseConnectorTest.java | 25 ++++++------------- 2 files changed, 7 insertions(+), 27 deletions(-) diff --git a/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/AbstractKuduConnectorTest.java b/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/AbstractKuduConnectorTest.java index e57f1f2ed032..094913f98bc8 100644 --- a/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/AbstractKuduConnectorTest.java +++ b/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/AbstractKuduConnectorTest.java @@ -339,15 +339,6 @@ public void testDelete() throw new SkipException("TODO"); } - @Test - @Override - public void testDeleteWithLike() - { - // TODO Support these test once kudu connector can create tables with default partitions - assertThatThrownBy(super::testDeleteWithLike) - .hasMessage("Table partitioning must be specified using setRangePartitionColumns or addHashPartitions"); - } - @Test @Override public void testDeleteWithSemiJoin() diff --git a/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java b/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java index 9b8b18a32bd8..70a5f8e0b95d 100644 --- a/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java +++ b/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java @@ -2453,7 +2453,7 @@ public void testInsertInTransaction() @Test public void testDelete() { - skipTestUnlessSupportsDeletes(); + skipTestUnless(hasBehavior(SUPPORTS_DELETE)); // delete successive parts of the table try (TestTable table = new TestTable(getQueryRunner()::execute, "test_delete_", "AS SELECT * FROM orders")) { @@ -2497,7 +2497,7 @@ public void testDelete() @Test public void testDeleteWithLike() { - skipTestUnlessSupportsDeletes(); + skipTestUnless(hasBehavior(SUPPORTS_DELETE)); try (TestTable table = new TestTable(getQueryRunner()::execute, "test_with_like_", "AS SELECT * FROM nation")) { assertUpdate("DELETE FROM " + table.getName() + " WHERE name LIKE '%a%'", "VALUES 0"); @@ -2508,7 +2508,7 @@ public void testDeleteWithLike() @Test public void testDeleteWithComplexPredicate() { - skipTestUnlessSupportsDeletes(); + skipTestUnless(hasBehavior(SUPPORTS_DELETE)); // TODO (https://github.com/trinodb/trino/issues/5901) Use longer table name once Oracle version is updated try (TestTable table = new TestTable(getQueryRunner()::execute, "test_delete_complex_", "AS SELECT * FROM orders")) { @@ -2526,7 +2526,7 @@ public void testDeleteWithComplexPredicate() @Test public void testDeleteWithSubquery() { - skipTestUnlessSupportsDeletes(); + skipTestUnless(hasBehavior(SUPPORTS_DELETE)); // TODO (https://github.com/trinodb/trino/issues/5901) Use longer table name once Oracle version is updated try (TestTable table = new TestTable(getQueryRunner()::execute, "test_delete_subquery", "AS SELECT * FROM nation")) { @@ -2550,7 +2550,7 @@ public void testDeleteWithSubquery() @Test public void testExplainAnalyzeWithDeleteWithSubquery() { - skipTestUnlessSupportsDeletes(); + skipTestUnless(hasBehavior(SUPPORTS_DELETE)); String tableName = "test_delete_" + randomTableSuffix(); @@ -2564,7 +2564,7 @@ public void testExplainAnalyzeWithDeleteWithSubquery() @Test public void testDeleteWithSemiJoin() { - skipTestUnlessSupportsDeletes(); + skipTestUnless(hasBehavior(SUPPORTS_DELETE)); // TODO (https://github.com/trinodb/trino/issues/5901) Use longer table name once Oracle version is updated try (TestTable table = new TestTable(getQueryRunner()::execute, "test_delete_semijoin", "AS SELECT * FROM nation")) { @@ -2599,7 +2599,7 @@ public void testDeleteWithSemiJoin() @Test public void testDeleteWithVarcharPredicate() { - skipTestUnlessSupportsDeletes(); + skipTestUnless(hasBehavior(SUPPORTS_DELETE)); try (TestTable table = new TestTable(getQueryRunner()::execute, "test_delete_with_varchar_predicate_", "AS SELECT * FROM orders")) { assertUpdate("DELETE FROM " + table.getName() + " WHERE orderstatus = 'O'", "SELECT count(*) FROM orders WHERE orderstatus = 'O'"); @@ -2607,17 +2607,6 @@ public void testDeleteWithVarcharPredicate() } } - protected void skipTestUnlessSupportsDeletes() - { - skipTestUnless(hasBehavior(SUPPORTS_CREATE_TABLE)); - try (TestTable table = new TestTable(getQueryRunner()::execute, "test_supports_delete", "(col varchar(1))")) { - if (!hasBehavior(SUPPORTS_DELETE)) { - assertQueryFails("DELETE FROM " + table.getName(), "This connector does not support deletes"); - throw new SkipException("This connector does not support deletes"); - } - } - } - @Test public void verifySupportsDeleteDeclaration() { From 83e3d58793e7769ae52c6581dec1b0d89d526226 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Thu, 21 Apr 2022 15:40:19 +0200 Subject: [PATCH 2/2] Use TestTable's builtin data setup --- .../TestPostgreSqlConnectorTest.java | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/plugin/trino-postgresql/src/test/java/io/trino/plugin/postgresql/TestPostgreSqlConnectorTest.java b/plugin/trino-postgresql/src/test/java/io/trino/plugin/postgresql/TestPostgreSqlConnectorTest.java index 5019ea77d8c5..77954a673703 100644 --- a/plugin/trino-postgresql/src/test/java/io/trino/plugin/postgresql/TestPostgreSqlConnectorTest.java +++ b/plugin/trino-postgresql/src/test/java/io/trino/plugin/postgresql/TestPostgreSqlConnectorTest.java @@ -632,10 +632,11 @@ public void testStringJoinPushdownWithCollate() @Test public void testDecimalPredicatePushdown() { - try (TestTable table = new TestTable(onRemoteDatabase(), "test_decimal_pushdown", - "(short_decimal decimal(9, 3), long_decimal decimal(30, 10))")) { - onRemoteDatabase().execute("INSERT INTO " + table.getName() + " VALUES (123.321, 123456789.987654321)"); - + try (TestTable table = new TestTable( + onRemoteDatabase(), + "test_decimal_pushdown", + "(short_decimal decimal(9, 3), long_decimal decimal(30, 10))", + List.of("123.321, 123456789.987654321"))) { assertThat(query("SELECT * FROM " + table.getName() + " WHERE short_decimal <= 124")) .matches("VALUES (CAST(123.321 AS decimal(9,3)), CAST(123456789.987654321 AS decimal(30, 10)))") .isFullyPushedDown(); @@ -663,12 +664,13 @@ public void testDecimalPredicatePushdown() @Test public void testCharPredicatePushdown() { - try (TestTable table = new TestTable(onRemoteDatabase(), "test_char_pushdown", - "(char_1 char(1), char_5 char(5), char_10 char(10))")) { - onRemoteDatabase().execute("INSERT INTO " + table.getName() + " VALUES" + - "('0', '0' , '0' )," + - "('1', '12345', '1234567890')"); - + try (TestTable table = new TestTable( + onRemoteDatabase(), + "test_char_pushdown", + "(char_1 char(1), char_5 char(5), char_10 char(10))", + List.of( + "'0', '0', '0'", + "'1', '12345', '1234567890'"))) { assertThat(query("SELECT * FROM " + table.getName() + " WHERE char_1 = '0' AND char_5 = '0'")) .matches("VALUES (CHAR'0', CHAR'0 ', CHAR'0 ')") .isFullyPushedDown();