Skip to content
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

Migrate to BaseConnectorTest in BigQuery #11108

Merged
merged 5 commits into from
Feb 25, 2022
Merged

Migrate to BaseConnectorTest in BigQuery #11108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e2ca2c6
Don't insert rows in skipTestUnlessSupportsDeletes
ebyhr Feb 19, 2022
13690df
Create an empty table in testQueryLoggingCount
ebyhr Feb 19, 2022
8b9ce67
Make AbstractTestDistributedQueries.testColumnName protected
ebyhr Feb 25, 2022
764c566
Rename TestBigQueryIntegrationSmokeTest to TestBigQueryConnectorTest
ebyhr Feb 19, 2022
d6b0159
Migrate to BaseConnectorTest in BigQuery
ebyhr Feb 19, 2022
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
4 changes: 2 additions & 2 deletions plugin/trino-bigquery/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@
<configuration>
<excludes>
<!-- If you are adding entry here also add an entry to cloud-tests or cloud-tests-case-insensitive-mapping profile below -->
<exclude>**/TestBigQueryIntegrationSmokeTest.java</exclude>
<exclude>**/TestBigQueryConnectorTest.java</exclude>
<exclude>**/TestBigQueryTypeMapping.java</exclude>
<exclude>**/TestBigQueryMetadata.java</exclude>
<exclude>**/TestBigQueryInstanceCleaner.java</exclude>
Expand All @@ -375,7 +375,7 @@
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<includes>
<include>**/TestBigQueryIntegrationSmokeTest.java</include>
<include>**/TestBigQueryConnectorTest.java</include>
<include>**/TestBigQueryTypeMapping.java</include>
<include>**/TestBigQueryMetadata.java</include>
<include>**/TestBigQueryInstanceCleaner.java</include>
Expand Down
208 changes: 204 additions & 4 deletions ...ery/TestBigQueryIntegrationSmokeTest.java → ...n/bigquery/TestBigQueryConnectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@
*/
package io.trino.plugin.bigquery;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import io.trino.testing.AbstractTestIntegrationSmokeTest;
import io.trino.testing.BaseConnectorTest;
import io.trino.testing.MaterializedResult;
import io.trino.testing.QueryRunner;
import io.trino.testing.TestingConnectorBehavior;
import io.trino.testing.sql.TestTable;
import io.trino.testing.sql.TestView;
import org.testng.annotations.BeforeClass;
Expand All @@ -25,19 +27,21 @@

import java.util.List;

import static com.google.common.base.Strings.nullToEmpty;
import static io.trino.plugin.bigquery.BigQueryQueryRunner.BigQuerySqlExecutor;
import static io.trino.spi.type.VarcharType.VARCHAR;
import static io.trino.testing.MaterializedResult.resultBuilder;
import static io.trino.testing.assertions.Assert.assertEquals;
import static io.trino.testing.sql.TestTable.randomTableSuffix;
import static java.lang.String.format;
import static java.util.Locale.ENGLISH;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertTrue;

public class TestBigQueryIntegrationSmokeTest
// TODO extend BaseConnectorTest
extends AbstractTestIntegrationSmokeTest
public class TestBigQueryConnectorTest
extends BaseConnectorTest
{
protected BigQuerySqlExecutor bigQuerySqlExecutor;

Expand All @@ -56,7 +60,30 @@ protected QueryRunner createQueryRunner()
ImmutableMap.of());
}

@Override
protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior)
{
switch (connectorBehavior) {
case SUPPORTS_TOPN_PUSHDOWN:
case SUPPORTS_RENAME_SCHEMA:
case SUPPORTS_RENAME_TABLE:
case SUPPORTS_NOT_NULL_CONSTRAINT:
case SUPPORTS_CREATE_TABLE_WITH_DATA:
case SUPPORTS_DELETE:
case SUPPORTS_INSERT:
case SUPPORTS_ADD_COLUMN:
case SUPPORTS_DROP_COLUMN:
case SUPPORTS_RENAME_COLUMN:
case SUPPORTS_COMMENT_ON_TABLE:
case SUPPORTS_COMMENT_ON_COLUMN:
return false;
default:
return super.hasBehavior(connectorBehavior);
}
}

@Test
@Override
public void testCreateSchema()
{
String schemaName = "test_schema_create_" + randomTableSuffix();
Expand All @@ -82,6 +109,28 @@ public void testCreateSchema()
assertUpdate("DROP SCHEMA IF EXISTS " + schemaName);
}

@Test
@Override
public void testShowColumns()
{
// shippriority column is bigint (not integer) in BigQuery connector
MaterializedResult actual = computeActual("SHOW COLUMNS FROM orders");

MaterializedResult expectedParametrizedVarchar = resultBuilder(getSession(), VARCHAR, VARCHAR, VARCHAR, VARCHAR)
.row("orderkey", "bigint", "", "")
.row("custkey", "bigint", "", "")
.row("orderstatus", "varchar", "", "")
.row("totalprice", "double", "", "")
.row("orderdate", "date", "", "")
.row("orderpriority", "varchar", "", "")
.row("clerk", "varchar", "", "")
.row("shippriority", "bigint", "", "")
.row("comment", "varchar", "", "")
.build();

assertEquals(actual, expectedParametrizedVarchar);
}

@Override
public void testDescribeTable()
{
Expand Down Expand Up @@ -181,6 +230,22 @@ public void testCreateTableIfNotExists()
}
}

@Test
@Override
public void testCreateTableAsSelect()
{
assertThatThrownBy(super::testCreateTableAsSelect)
.hasStackTraceContaining("This connector does not support creating tables with data");
}

@Test
@Override
public void testCreateTableAsSelectWithUnicode()
{
assertThatThrownBy(super::testCreateTableAsSelectWithUnicode)
.hasStackTraceContaining("This connector does not support creating tables with data");
}

@Test
public void testDropTable()
{
Expand All @@ -192,6 +257,80 @@ public void testDropTable()
assertFalse(getQueryRunner().tableExists(getSession(), tableName));
}

@Test
@Override
public void testRenameTable()
{
// Use CREATE TABLE instead of CREATE TABLE AS statement
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we fix this in base test? It seems we don't need CTAS to test rename table either?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could do it, but I hesitated to change in this PR because the test contains assertion for values.

String tableName = "test_rename_" + randomTableSuffix();
assertUpdate("CREATE TABLE " + tableName + " (x int)");

String renamedTable = "test_rename_new_" + randomTableSuffix();
assertQueryFails("ALTER TABLE " + tableName + " RENAME TO " + renamedTable, "This connector does not support renaming tables");
}

@Test(dataProvider = "testDataMappingSmokeTestDataProvider")
@Override
public void testDataMappingSmokeTest(DataMappingTestSetup dataMappingTestSetup)
{
assertThatThrownBy(() -> super.testDataMappingSmokeTest(dataMappingTestSetup))
.hasMessageContaining("This connector does not support creating tables with data");
}

@Test(dataProvider = "testCaseSensitiveDataMappingProvider")
@Override
public void testCaseSensitiveDataMapping(DataMappingTestSetup dataMappingTestSetup)
{
assertThatThrownBy(() -> super.testCaseSensitiveDataMapping(dataMappingTestSetup))
.hasMessageContaining("This connector does not support creating tables with data");
}

@Override
protected void testColumnName(String columnName, boolean delimited)
{
// Override because BigQuery connector doesn't support INSERT statement
String nameInSql = columnName;
if (delimited) {
nameInSql = "\"" + columnName.replace("\"", "\"\"") + "\"";
}
String tableName = "test.tcn_" + nameInSql.toLowerCase(ENGLISH).replaceAll("[^a-z0-9]", "") + randomTableSuffix();

try {
// TODO test with both CTAS *and* CREATE TABLE + INSERT, since they use different connector API methods.
assertUpdate("CREATE TABLE " + tableName + "(key varchar(50), " + nameInSql + " varchar(50))");
}
catch (RuntimeException e) {
if (isColumnNameRejected(e, columnName, delimited)) {
// It is OK if give column name is not allowed and is clearly rejected by the connector.
return;
}
throw e;
}
try {
// Execute INSERT statement in BigQuery
onBigQuery("INSERT INTO " + tableName + " VALUES ('null value', NULL), ('sample value', 'abc'), ('other value', 'xyz')");

// SELECT *
assertQuery("SELECT * FROM " + tableName, "VALUES ('null value', NULL), ('sample value', 'abc'), ('other value', 'xyz')");

// projection
assertQuery("SELECT " + nameInSql + " FROM " + tableName, "VALUES (NULL), ('abc'), ('xyz')");

// predicate
assertQuery("SELECT key FROM " + tableName + " WHERE " + nameInSql + " IS NULL", "VALUES ('null value')");
assertQuery("SELECT key FROM " + tableName + " WHERE " + nameInSql + " = 'abc'", "VALUES ('sample value')");
}
finally {
assertUpdate("DROP TABLE " + tableName);
}
}

@Override
protected boolean isColumnNameRejected(Exception exception, String columnName, boolean delimited)
{
return nullToEmpty(exception.getMessage()).matches(".*(Fields must contain only letters, numbers, and underscores, start with a letter or underscore, and be at most 300 characters long).*");
}

@Test(enabled = false)
public void testSelectFromHourlyPartitionedTable()
{
Expand Down Expand Up @@ -323,6 +462,46 @@ public void testShowCreateTable()
")");
}

@Test
@Override
public void testCharVarcharComparison()
{
// BigQuery doesn't have char type
assertThatThrownBy(super::testCharVarcharComparison)
.hasMessage("This connector does not support creating tables with data");
}

@Test
@Override
public void testVarcharCharComparison()
{
// Use BigQuery SQL executor because the connector doesn't support INSERT statement
try (TestTable table = new TestTable(
bigQuerySqlExecutor,
"test.test_varchar_char",
"(k int, v string(3))",
ImmutableList.of(
"-1, NULL",
"0, ''",
"1, ' '",
"2, ' '",
"3, ' '",
"4, 'x'",
"5, 'x '",
"6, 'x '"))) {
assertQuery(
"SELECT k, v FROM " + table.getName() + " WHERE v = CAST(' ' AS char(2))",
// The 3-spaces value is included because both sides of the comparison are coerced to char(3)
"VALUES (0, ''), (1, ' '), (2, ' '), (3, ' ')");

// value that's not all-spaces
assertQuery(
"SELECT k, v FROM " + table.getName() + " WHERE v = CAST('x ' AS char(2))",
// The 3-spaces value is included because both sides of the comparison are coerced to char(3)
"VALUES (4, 'x'), (5, 'x '), (6, 'x ')");
}
}

@Test
public void testSkipUnsupportedType()
{
Expand All @@ -340,6 +519,27 @@ public void testSkipUnsupportedType()
}
}

@Test
@Override
public void testDateYearOfEraPredicate()
{
// Override because the connector throws an exception instead of an empty result when the value is out of supported range
assertQuery("SELECT orderdate FROM orders WHERE orderdate = DATE '1997-09-14'", "VALUES DATE '1997-09-14'");
assertThatThrownBy(() -> query("SELECT * FROM orders WHERE orderdate = DATE '-1996-09-14'"))
.hasMessageMatching(".*Row filter for .* is invalid\\. Filter is '\\(`orderdate` = '-1996-09-14'\\)'");
}

@Test
@Override
public void testSymbolAliasing()
{
// Create table in BigQuery because the connector doesn't support CREATE TABLE AS SELECT statement
String tableName = "test.test_symbol_aliasing" + randomTableSuffix();
onBigQuery("CREATE TABLE " + tableName + " AS SELECT 1 foo_1, 2 foo_2_4");
assertQuery("SELECT foo_1, foo_2_4 FROM " + tableName, "SELECT 1, 2");
assertUpdate("DROP TABLE " + tableName);
}

private void onBigQuery(String sql)
{
bigQuerySqlExecutor.execute(sql);
Expand Down
24 changes: 14 additions & 10 deletions plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/AbstractKuduConnectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,17 +189,8 @@ public void testRowDelete()
assertUpdate("DROP TABLE test_row_delete");
}

@Test(dataProvider = "testColumnNameDataProvider")
@Override
public void testColumnName(String columnName)
{
if (!requiresDelimiting(columnName)) {
testColumnName(columnName, false);
}
testColumnName(columnName, true);
}

private void testColumnName(String columnName, boolean delimited)
protected void testColumnName(String columnName, boolean delimited)
{
String nameInSql = columnName;
if (delimited) {
Expand Down Expand Up @@ -432,6 +423,19 @@ protected TestTable createTableWithDefaultColumns()
throw new SkipException("Kudu connector does not support column default values");
}

@Override
protected String tableDefinitionForQueryLoggingCount()
{
return "( " +
" foo_1 int WITH (primary_key=true), " +
" foo_2_4 int " +
") " +
"WITH ( " +
" partition_by_hash_columns = ARRAY['foo_1'], " +
" partition_by_hash_buckets = 2 " +
")";
}

private void assertTableProperty(String tableProperties, String key, String regexValue)
{
assertTrue(Pattern.compile(key + "\\s*=\\s*" + regexValue + ",?\\s+").matcher(tableProperties).find(),
Expand Down
17 changes: 13 additions & 4 deletions testing/trino-testing/src/main/java/io/trino/testing/AbstractTestDistributedQueries.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -771,7 +771,7 @@ public void testDeleteWithVarcharPredicate()
protected void skipTestUnlessSupportsDeletes()
{
skipTestUnless(supportsCreateTable());
try (TestTable table = new TestTable(getQueryRunner()::execute, "test_supports_delete", "(col varchar(1))", ImmutableList.of("'a'", "'A'"))) {
try (TestTable table = new TestTable(getQueryRunner()::execute, "test_supports_delete", "(col varchar(1))")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I was testing for CREATE TABLE but performing CTAS in the test.

if (!supportsDelete()) {
assertQueryFails("DELETE FROM " + table.getName(), "This connector does not support deletes");
throw new SkipException("This connector does not support deletes");
Expand Down Expand Up @@ -1035,8 +1035,8 @@ public void testQueryLoggingCount()
long beforeCompletedQueriesCount = waitUntilStable(() -> dispatchManager.getStats().getCompletedQueries().getTotalCount(), new Duration(5, SECONDS));
long beforeSubmittedQueriesCount = dispatchManager.getStats().getSubmittedQueries().getTotalCount();
String tableName = "test_logging_count" + randomTableSuffix();
assertUpdate("CREATE TABLE " + tableName + " AS SELECT 1 foo_1, 2 foo_2_4", 1);
assertQuery("SELECT foo_1, foo_2_4 FROM " + tableName, "SELECT 1, 2");
assertUpdate("CREATE TABLE " + tableName + tableDefinitionForQueryLoggingCount());
assertQueryReturnsEmptyResult("SELECT foo_1, foo_2_4 FROM " + tableName);
assertUpdate("DROP TABLE " + tableName);
assertQueryFails("SELECT * FROM " + tableName, ".*Table .* does not exist");

Expand All @@ -1048,6 +1048,15 @@ public void testQueryLoggingCount()
});
}

/**
* The table must have two columns foo_1 and foo_2_4 of any type.
*/
@Language("SQL")
protected String tableDefinitionForQueryLoggingCount()
{
return "(foo_1 int, foo_2_4 int)";
}

private <T> T waitUntilStable(Supplier<T> computation, Duration timeout)
{
T lastValue = computation.get();
Expand Down Expand Up @@ -1197,7 +1206,7 @@ public void testColumnName(String columnName)
testColumnName(columnName, true);
}

private void testColumnName(String columnName, boolean delimited)
protected void testColumnName(String columnName, boolean delimited)
{
String nameInSql = columnName;
if (delimited) {
Expand Down
4 changes: 2 additions & 2 deletions testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1296,7 +1296,7 @@ public void verifySupportsDeleteDeclaration()
}

skipTestUnless(hasBehavior(SUPPORTS_CREATE_TABLE));
try (TestTable table = new TestTable(getQueryRunner()::execute, "test_supports_delete", "AS SELECT * FROM region")) {
try (TestTable table = new TestTable(getQueryRunner()::execute, "test_supports_delete", "(regionkey int)")) {
assertQueryFails("DELETE FROM " + table.getName(), "This connector does not support deletes");
}
}
Expand All @@ -1310,7 +1310,7 @@ public void verifySupportsRowLevelDeleteDeclaration()
}

skipTestUnless(hasBehavior(SUPPORTS_CREATE_TABLE));
try (TestTable table = new TestTable(getQueryRunner()::execute, "test_supports_row_level_delete", "AS SELECT * FROM region")) {
try (TestTable table = new TestTable(getQueryRunner()::execute, "test_supports_row_level_delete", "(regionkey int)")) {
assertQueryFails("DELETE FROM " + table.getName() + " WHERE regionkey = 2", "This connector does not support deletes");
}
}
Expand Down