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.

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

Enable testColumnName in TestCassandraDistributedQueries #3623

Merged
merged 2 commits into from
May 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import static io.prestosql.plugin.cassandra.CassandraType.toCassandraType;
import static io.prestosql.plugin.cassandra.util.CassandraCqlUtils.ID_COLUMN_NAME;
import static io.prestosql.plugin.cassandra.util.CassandraCqlUtils.cqlNameToSqlName;
import static io.prestosql.plugin.cassandra.util.CassandraCqlUtils.quoteStringLiteral;
import static io.prestosql.plugin.cassandra.util.CassandraCqlUtils.validColumnName;
import static io.prestosql.spi.StandardErrorCode.NOT_SUPPORTED;
import static io.prestosql.spi.StandardErrorCode.PERMISSION_DENIED;
Expand Down Expand Up @@ -310,7 +311,7 @@ private CassandraOutputTableHandle createTable(ConnectorTableMetadata tableMetad

// encode column ordering in the cassandra table comment field since there is no better place to store this
String columnMetadata = extraColumnMetadataCodec.toJson(columnExtra.build());
queryBuilder.append("WITH comment='").append(PRESTO_COMMENT_METADATA).append(" ").append(columnMetadata).append("'");
queryBuilder.append("WITH comment=").append(quoteStringLiteral(PRESTO_COMMENT_METADATA + " " + columnMetadata));

// We need to create the Cassandra table before commit because the record needs to be written to the table.
cassandraSession.execute(queryBuilder.toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

import java.util.List;

import static com.datastax.driver.core.Metadata.quote;
import static java.nio.charset.StandardCharsets.UTF_8;

public final class CassandraCqlUtils
Expand All @@ -34,12 +35,12 @@ private CassandraCqlUtils() {}

public static String validSchemaName(String identifier)
{
return quoteIdentifier(identifier);
return quote(identifier);
}

public static String validTableName(String identifier)
{
return quoteIdentifier(identifier);
return quote(identifier);
}

public static String validColumnName(String identifier)
Expand All @@ -48,12 +49,7 @@ public static String validColumnName(String identifier)
return "\"\"";
}

return quoteIdentifier(identifier);
}

private static String quoteIdentifier(String identifier)
{
return '"' + identifier + '"';
return quote(identifier);
}

public static String quoteStringLiteral(String string)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,16 +150,6 @@ protected TestTable createTableWithDefaultColumns()
throw new SkipException("Cassandra connector does not support column default values");
}

@Override
public void testColumnName(String columnName)
Copy link
Member

Choose a reason for hiding this comment

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

Enable testColumnName in TestCassandraDistributedQueries

Consider squashing this commit with Use Metadata.quote in CassandraCqlUtils -- if this is what fixes Cassandra and makes this test pass now.

{
// TODO Enable after fixing the following error messages
// - Multiple definition of identifier id
// - Column family names shouldn't be more than 48 characters long
// - mismatched character '<EOF>'
// - missing EOF at 'apostrophe'
}

@Override
public void testDataMappingSmokeTest(DataMappingTestSetup dataMappingTestSetup)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1180,7 +1180,7 @@ private void testColumnName(String columnName, boolean delimited)

try {
// TODO test with both CTAS *and* CREATE TABLE + INSERT, since they use different connector API methods.
assertUpdate("CREATE TABLE " + tableName + "(id varchar, " + nameInSql + " varchar)");
assertUpdate("CREATE TABLE " + tableName + "(key varchar, " + nameInSql + " varchar)");
Copy link
Member

Choose a reason for hiding this comment

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

Rename id column to key in testColumnName

in the commit message maybe mention that id has a special meaning in Cassandra.

}
catch (RuntimeException e) {
if (isColumnNameRejected(e, columnName, delimited)) {
Expand All @@ -1198,8 +1198,8 @@ private void testColumnName(String columnName, boolean delimited)
assertQuery("SELECT " + nameInSql + " FROM " + tableName, "VALUES (NULL), ('abc'), ('xyz')");

// predicate
assertQuery("SELECT id FROM " + tableName + " WHERE " + nameInSql + " IS NULL", "VALUES ('null value')");
assertQuery("SELECT id FROM " + tableName + " WHERE " + nameInSql + " = 'abc'", "VALUES ('sample value')");
assertQuery("SELECT key FROM " + tableName + " WHERE " + nameInSql + " IS NULL", "VALUES ('null value')");
assertQuery("SELECT key FROM " + tableName + " WHERE " + nameInSql + " = 'abc'", "VALUES ('sample value')");

assertUpdate("DROP TABLE " + tableName);
}
Expand Down Expand Up @@ -1237,7 +1237,7 @@ public Object[][] testColumnNameDataProvider()
{"a/slash`"},
{"a\\backslash`"},
{"adigit0"},
{"0startingwithdigit"},
{"0startwithdigit"},
};
}

Expand Down