Skip to content

Fix TestRedshiftConnectorTest and deny adding not columns explicitly #17381

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
merged 2 commits into from
May 11, 2023
Merged

Fix TestRedshiftConnectorTest and deny adding not columns explicitly #17381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
989e624
Fix TestRedshiftConnectorTest TestingConnectorBehavior
ebyhr May 10, 2023
22c0b2c
Deny adding a not null column in Redshift connector
ebyhr May 10, 2023
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
13 changes: 13 additions & 0 deletions plugin/trino-redshift/src/main/java/io/trino/plugin/redshift/RedshiftClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import io.trino.plugin.jdbc.ObjectWriteFunction;
import io.trino.plugin.jdbc.PreparedQuery;
import io.trino.plugin.jdbc.QueryBuilder;
import io.trino.plugin.jdbc.RemoteTableName;
import io.trino.plugin.jdbc.SliceWriteFunction;
import io.trino.plugin.jdbc.StandardColumnMappings;
import io.trino.plugin.jdbc.WriteMapping;
Expand All @@ -58,6 +59,7 @@
import io.trino.spi.TrinoException;
import io.trino.spi.connector.AggregateFunction;
import io.trino.spi.connector.ColumnHandle;
import io.trino.spi.connector.ColumnMetadata;
import io.trino.spi.connector.ConnectorSession;
import io.trino.spi.connector.JoinCondition;
import io.trino.spi.connector.JoinStatistics;
Expand Down Expand Up @@ -442,6 +444,17 @@ public OptionalLong delete(ConnectorSession session, JdbcTableHandle handle)
}
}

@Override
protected void addColumn(ConnectorSession session, Connection connection, RemoteTableName table, ColumnMetadata column)
throws SQLException
{
if (!column.isNullable()) {
// Redshift doesn't support adding not null columns without default expression
throw new TrinoException(NOT_SUPPORTED, "This connector does not support adding not null columns");
}
super.addColumn(session, connection, table, column);
}

@Override
protected void verifySchemaName(DatabaseMetaData databaseMetadata, String schemaName)
throws SQLException
Expand Down
26 changes: 20 additions & 6 deletions plugin/trino-redshift/src/test/java/io/trino/plugin/redshift/TestRedshiftConnectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,16 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior)
{
switch (connectorBehavior) {
case SUPPORTS_COMMENT_ON_TABLE:
return false;
case SUPPORTS_COMMENT_ON_COLUMN:
return true;

case SUPPORTS_CREATE_TABLE_WITH_TABLE_COMMENT:
case SUPPORTS_CREATE_TABLE_WITH_COLUMN_COMMENT:
return false;

case SUPPORTS_ADD_COLUMN_WITH_COMMENT:
case SUPPORTS_ADD_COLUMN_NOT_NULL_CONSTRAINT:
case SUPPORTS_SET_COLUMN_TYPE:
return false;

Expand All @@ -82,10 +87,10 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior)
case SUPPORTS_RENAME_TABLE_ACROSS_SCHEMAS:
return false;

case SUPPORTS_AGGREGATION_PUSHDOWN_STDDEV:
case SUPPORTS_AGGREGATION_PUSHDOWN_VARIANCE:
case SUPPORTS_AGGREGATION_PUSHDOWN_COUNT_DISTINCT:
return true;
case SUPPORTS_AGGREGATION_PUSHDOWN_COVARIANCE:
case SUPPORTS_AGGREGATION_PUSHDOWN_CORRELATION:
case SUPPORTS_AGGREGATION_PUSHDOWN_REGRESSION:
return false;

case SUPPORTS_JOIN_PUSHDOWN:
case SUPPORTS_JOIN_PUSHDOWN_WITH_VARCHAR_EQUALITY:
Expand Down Expand Up @@ -194,6 +199,15 @@ public Object[][] redshiftTypeToTrinoTypes()
{"TIMESTAMPTZ", "timestamp(6) with time zone"}};
}

@Test
public void testRedshiftAddNotNullColumn()
{
try (TestTable table = new TestTable(getQueryRunner()::execute, TEST_SCHEMA + ".test_add_column_", "(col int)")) {
assertThatThrownBy(() -> onRemoteDatabase().execute("ALTER TABLE " + table.getName() + " ADD COLUMN new_col int NOT NULL"))
.hasMessageContaining("ERROR: ALTER TABLE ADD COLUMN defined as NOT NULL must have a non-null default expression");
}
}

@Override
public void testDelete()
{
Expand Down Expand Up @@ -536,9 +550,9 @@ public void testDecimalAvgPushdownForMaximumDecimalScale()
.isInstanceOf(AssertionError.class)
.hasMessageContaining("""
elements not found:
<(555555555555555555561728450.9938271605)>
(555555555555555555561728450.9938271605)
and elements not expected:
<(555555555555555555561728450.9938271604)>
(555555555555555555561728450.9938271604)
""");
}
}
Expand Down