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

FIR-33264: quoted engine and DB names #417

Merged
merged 3 commits into from
May 28, 2024
Merged

FIR-33264: quoted engine and DB names #417

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5377f04
FIR-33264: quoted engine, DB and table names in DML statements
May 28, 2024
f6602b0
fixed broken useEngine() test
May 28, 2024
b7817a4
added more integration tests with mixed case engine name
May 28, 2024
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
2 changes: 1 addition & 1 deletion src/integrationTest/java/integration/tests/ConnectionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ void connectToWrongDbNotAttachedToEngine() throws SQLException {
String enginelessDb = "engineless_db" + System.currentTimeMillis();
try (Connection systemConnection = createConnection(null)) {
try {
systemConnection.createStatement().executeUpdate(format("CREATE DATABASE IF NOT EXISTS %s", enginelessDb));
systemConnection.createStatement().executeUpdate(format("CREATE DATABASE IF NOT EXISTS \"%s\"", enginelessDb));
String url = format("jdbc:firebolt:%s?env=%s&account=%s&engine=%s", enginelessDb, params.getEnv(), params.getAccount(), params.getEngine());
String errorMessage = format("The engine with the name %s is not attached to database %s", params.getEngine(), enginelessDb);
assertEquals(errorMessage, assertThrows(FireboltException.class, () -> DriverManager.getConnection(url, params.getPrincipal(), params.getSecret())).getMessage());
Expand Down
99 changes: 71 additions & 28 deletions src/integrationTest/java/integration/tests/SystemEngineTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import java.util.logging.Logger;
import java.util.stream.Stream;

import static com.firebolt.jdbc.connection.FireboltConnectionUserPassword.SYSTEM_ENGINE_NAME;
import static java.lang.String.format;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Map.entry;
Expand Down Expand Up @@ -134,23 +135,23 @@ void useDatabase(String entityType) throws SQLException {
ConnectionInfo current = integration.ConnectionInfo.getInstance();
try (Connection connection = createConnection(getSystemEngineName())) {
try {
connection.createStatement().executeUpdate(format("USE %s %s", entityType, current.getDatabase())); // use current DB; shouldn't have any effect
connection.createStatement().executeUpdate(format("USE %s \"%s\"", entityType, current.getDatabase())); // use current DB; shouldn't have any effect
assertNull(getTableDbName(connection, TABLE1)); // the table does not exist yet
connection.createStatement().executeUpdate(format("CREATE TABLE %s ( id LONG)", TABLE1)); // create table1 in current DB
connection.createStatement().executeUpdate(format("CREATE TABLE \"%s\" ( id LONG)", TABLE1)); // create table1 in current DB
assertEquals(current.getDatabase(), getTableDbName(connection, TABLE1)); // now table t1 exists
Assert.assertThrows(SQLException.class, () -> connection.createStatement().executeUpdate(format("USE %s %s", entityType, USE_DATABASE_NAME))); // DB does not exist
connection.createStatement().executeUpdate(format("CREATE DATABASE IF NOT EXISTS %s", USE_DATABASE_NAME)); // create DB
connection.createStatement().executeUpdate(format("CREATE DATABASE IF NOT EXISTS \"%s\"", USE_DATABASE_NAME)); // create DB
connection.createStatement().executeUpdate(format("USE %s %s", entityType, USE_DATABASE_NAME)); // Now this should succeed
connection.createStatement().executeUpdate(format("CREATE TABLE %s ( id LONG)", TABLE2)); // create table2 in other DB
connection.createStatement().executeUpdate(format("CREATE TABLE \"%s\" ( id LONG)", TABLE2)); // create table2 in other DB
assertNull(getTableDbName(connection, TABLE1)); // table1 does not exist here
assertEquals(USE_DATABASE_NAME, getTableDbName(connection, TABLE2)); // but table2 does exist
} finally {
// now clean up everything
for (String query : new String[] {
format("USE %s %s", entityType, USE_DATABASE_NAME), // switch to DB that should be current just in case because the previous code can fail at any phase
format("USE %s \"%s\"", entityType, USE_DATABASE_NAME), // switch to DB that should be current just in case because the previous code can fail at any phase
format("DROP TABLE %s", TABLE2),
format("DROP DATABASE %s", USE_DATABASE_NAME),
format("USE %s %s", entityType, current.getDatabase()), // now switch back
format("DROP DATABASE \"%s\"", USE_DATABASE_NAME),
format("USE %s \"%s\"", entityType, current.getDatabase()), // now switch back
format("DROP TABLE %s", TABLE1)}) {
try (Statement statement = connection.createStatement()) {
statement.executeUpdate(query);
Expand All @@ -169,23 +170,65 @@ void useDatabase(String entityType) throws SQLException {
void useEngine() throws SQLException {
try (Connection connection = createConnection(getSystemEngineName())) {
try {
connection.createStatement().executeUpdate("USE ENGINE SYSTEM");
assertThrows(SQLException.class, () -> connection.createStatement().executeUpdate(format("USE ENGINE %s", ENGINE_NAME)));
connection.createStatement().executeUpdate(format("CREATE ENGINE %s", ENGINE_NAME));
connection.createStatement().executeUpdate(format("USE ENGINE %s", ENGINE_NAME));
connection.createStatement().executeUpdate(format("CREATE DATABASE IF NOT EXISTS %s", USE_DATABASE_NAME));
connection.createStatement().executeUpdate(format("USE DATABASE %s", USE_DATABASE_NAME));
connection.createStatement().executeUpdate(format("CREATE TABLE %s ( id LONG)", TABLE1));
connection.createStatement().executeUpdate(format("USE ENGINE \"%s\"", SYSTEM_ENGINE_NAME));
assertThrows(SQLException.class, () -> connection.createStatement().executeUpdate(format("USE ENGINE \"%s\"", ENGINE_NAME)));
connection.createStatement().executeUpdate(format("CREATE ENGINE \"%s\"", ENGINE_NAME));
connection.createStatement().executeUpdate(format("USE ENGINE \"%s\"", ENGINE_NAME));
connection.createStatement().executeUpdate(format("CREATE DATABASE IF NOT EXISTS \"%s\"", USE_DATABASE_NAME));
connection.createStatement().executeUpdate(format("USE DATABASE \"%s\"", USE_DATABASE_NAME));
connection.createStatement().executeUpdate(format("CREATE TABLE \"%s\" ( id LONG)", TABLE1));
connection.createStatement().executeUpdate(format("INSERT INTO %s (id) VALUES (1)", TABLE1)); // should succeed using user engine
// switch back to the system engine
connection.createStatement().executeUpdate("USE ENGINE SYSTEM");
connection.createStatement().executeUpdate(format("USE ENGINE \"%s\"", SYSTEM_ENGINE_NAME));
assertThrows(SQLException.class, () -> connection.createStatement().executeUpdate(format("INSERT INTO %s (id) VALUES (1)", TABLE1))); // system engine cannot insert data
} finally {
connection.createStatement().executeUpdate(format("USE DATABASE %s", USE_DATABASE_NAME));
connection.createStatement().executeUpdate(format("USE DATABASE \"%s\"", USE_DATABASE_NAME));
connection.createStatement().executeUpdate(format("DROP TABLE %s", TABLE1));
connection.createStatement().executeUpdate(format("DROP DATABASE %s", USE_DATABASE_NAME));
connection.createStatement().executeUpdate(format("STOP ENGINE %s", ENGINE_NAME));
connection.createStatement().executeUpdate(format("DROP ENGINE %s", ENGINE_NAME));
connection.createStatement().executeUpdate(format("DROP DATABASE \"%s\"", USE_DATABASE_NAME));
connection.createStatement().executeUpdate(format("STOP ENGINE \"%s\"", ENGINE_NAME));
connection.createStatement().executeUpdate(format("DROP ENGINE \"%s\"", ENGINE_NAME));
}
}
}

@Test
@Tag("v2")
@Tag("slow")
@EnvironmentCondition(value = "2", comparison = EnvironmentCondition.Comparison.GE)
void useEngineMixedCase() throws SQLException {
String mixedCaseEngineName = "JavaIntegrationTestMixedCase" + ID;
try (Connection connection = createConnection(getSystemEngineName())) {
try {
connection.createStatement().executeUpdate(format("USE ENGINE \"%s\"", SYSTEM_ENGINE_NAME));
connection.createStatement().executeUpdate(format("CREATE ENGINE \"%s\"", mixedCaseEngineName));
connection.createStatement().executeUpdate(format("USE ENGINE \"%s\"", mixedCaseEngineName));
assertThrows(SQLException.class, () -> connection.createStatement().executeUpdate(format("USE ENGINE %s", mixedCaseEngineName)));
} finally {
connection.createStatement().executeUpdate(format("USE ENGINE \"%s\"", SYSTEM_ENGINE_NAME));
connection.createStatement().executeUpdate(format("STOP ENGINE \"%s\"", mixedCaseEngineName));
connection.createStatement().executeUpdate(format("DROP ENGINE \"%s\"", mixedCaseEngineName));
}
}
}

@Test
@Tag("v2")
@Tag("slow")
@EnvironmentCondition(value = "2", comparison = EnvironmentCondition.Comparison.GE)
void useEngineMixedCaseToLowerCase() throws SQLException {
String mixedCaseEngineName = "JavaIntegrationTestToLowerCase" + ID;
try (Connection connection = createConnection(getSystemEngineName())) {
try {
connection.createStatement().executeUpdate(format("USE ENGINE \"%s\"", SYSTEM_ENGINE_NAME));
// engine name is lower cased because it is not quoted
connection.createStatement().executeUpdate(format("CREATE ENGINE %s", mixedCaseEngineName));
connection.createStatement().executeUpdate(format("USE ENGINE %s", mixedCaseEngineName));
// engine name remains mixed case and statement fails because engine name was not quoted when we created the engine
assertThrows(SQLException.class, () -> connection.createStatement().executeUpdate(format("USE ENGINE \"%s\"", mixedCaseEngineName)));
} finally {
connection.createStatement().executeUpdate(format("USE ENGINE \"%s\"", SYSTEM_ENGINE_NAME));
connection.createStatement().executeUpdate(format("STOP ENGINE %s", mixedCaseEngineName));
connection.createStatement().executeUpdate(format("DROP ENGINE %s", mixedCaseEngineName));
}
}
}
Expand Down Expand Up @@ -259,20 +302,20 @@ void shouldExecuteEngineManagementQueries() throws SQLException {
try {
boolean attachEngineToDb = ((FireboltConnection)connection).getInfraVersion() < 2;
List<String> queries = Stream.of(
entry(true, format("CREATE DATABASE IF NOT EXISTS %s", SECOND_DATABASE_NAME)),
entry(true, format("CREATE ENGINE %s", ENGINE_NAME)),
entry(attachEngineToDb, format("ATTACH ENGINE %s TO %s;", ENGINE_NAME, SECOND_DATABASE_NAME)),
entry(true, format("ALTER DATABASE %s SET DESCRIPTION = 'JDBC Integration test'", SECOND_DATABASE_NAME)),
entry(true, format("ALTER ENGINE %s RENAME TO %s", ENGINE_NAME, ENGINE_NEW_NAME)),
entry(true, format("START ENGINE %s", ENGINE_NEW_NAME)))
entry(true, format("CREATE DATABASE IF NOT EXISTS \"%s\"", SECOND_DATABASE_NAME)),
entry(true, format("CREATE ENGINE \"%s\"", ENGINE_NAME)),
entry(attachEngineToDb, format("ATTACH ENGINE \"%s\" TO \"%s\";", ENGINE_NAME, SECOND_DATABASE_NAME)),
entry(true, format("ALTER DATABASE \"%s\" SET DESCRIPTION = 'JDBC Integration test'", SECOND_DATABASE_NAME)),
entry(true, format("ALTER ENGINE \"%s\" RENAME TO \"%s\"", ENGINE_NAME, ENGINE_NEW_NAME)),
entry(true, format("START ENGINE \"%s\"", ENGINE_NEW_NAME)))
.filter(Map.Entry::getKey).map(Map.Entry::getValue).collect(toList());
executeAll(connection, queries);
} finally {
// now clean up everything
for (String query : new String[]{
format("STOP ENGINE %s", ENGINE_NEW_NAME),
format("DROP ENGINE %s", ENGINE_NEW_NAME),
format("DROP DATABASE %s", SECOND_DATABASE_NAME)}) {
format("STOP ENGINE \"%s\"", ENGINE_NEW_NAME),
format("DROP ENGINE \"%s\"", ENGINE_NEW_NAME),
format("DROP DATABASE \"%s\"", SECOND_DATABASE_NAME)}) {
try (Statement statement = connection.createStatement()) {
statement.executeUpdate(query);
} catch (SQLException e) { // catch just in case to do our best to clean everything even if test has failed
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/firebolt/jdbc/service/FireboltEngineVersion2Service.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ public Engine getEngine(FireboltProperties properties) throws SQLException {
}

private String use(String entity, String name) {
return format("USE %s %s", entity, name);
return format("USE %s \"%s\"", entity, name);
}
}
Loading