Skip to content

Commit

Permalink
JAVA-2037: Fix NPE when preparing statement with no bound variables
Browse files Browse the repository at this point in the history
  • Loading branch information
@olim7t
Alexandre Dutra authored and olim7t committed Nov 21, 2018
1 parent f345c5d commit 017053e
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 24 deletions.
1 change: 1 addition & 0 deletions changelog/README.md
View file
Expand Up @@ -4,6 +4,7 @@

### 4.0.0-beta3 (in progress)

- [bug] JAVA-2037: Fix NPE when preparing statement with no bound variables
- [improvement] JAVA-2014: Schedule timeouts on a separate Timer
- [bug] JAVA-2029: Handle schema refresh failure after a DDL query
- [bug] JAVA-1947: Make schema parsing more lenient and allow missing system_virtual_schema
Expand Down
10 changes: 9 additions & 1 deletion core/src/main/java/com/datastax/oss/driver/internal/core/cql/Conversions.java
View file
Expand Up @@ -362,7 +362,7 @@ public static DefaultPreparedStatement toPreparedStatement(
ByteBuffer.wrap(response.preparedQueryId).asReadOnlyBuffer(),
request.getQuery(),
toColumnDefinitions(response.variablesMetadata, context),
Ints.asList(response.variablesMetadata.pkIndices),
asList(response.variablesMetadata.pkIndices),
(response.resultMetadataId == null)
? null
: ByteBuffer.wrap(response.resultMetadataId).asReadOnlyBuffer(),
Expand Down Expand Up @@ -396,6 +396,14 @@ public static ColumnDefinitions toColumnDefinitions(
return DefaultColumnDefinitions.valueOf(ImmutableList.copyOf(values));
}

public static List<Integer> asList(int[] pkIndices) {
if (pkIndices == null || pkIndices.length == 0) {
return Collections.emptyList();
} else {
return Ints.asList(pkIndices);
}
}

public static CoordinatorException toThrowable(
Node node, Error errorMessage, InternalDriverContext context) {
switch (errorMessage.code) {
Expand Down
87 changes: 64 additions & 23 deletions .../cql/PreparedStatementInvalidationIT.java → ...ver/api/core/cql/PreparedStatementIT.java
View file
Expand Up @@ -51,11 +51,11 @@
* </pre>
*/
@Category(ParallelizableTests.class)
public class PreparedStatementInvalidationIT {
public class PreparedStatementIT {

private CcmRule ccmRule = CcmRule.getInstance();
private final CcmRule ccmRule = CcmRule.getInstance();

private SessionRule<CqlSession> sessionRule =
private final SessionRule<CqlSession> sessionRule =
SessionRule.builder(ccmRule)
.withConfigLoader(
SessionUtils.configLoaderBuilder()
Expand All @@ -72,11 +72,11 @@ public class PreparedStatementInvalidationIT {
public void setupSchema() {
for (String query :
ImmutableList.of(
"CREATE TABLE prepared_statement_invalidation_test (a int PRIMARY KEY, b int, c int)",
"INSERT INTO prepared_statement_invalidation_test (a, b, c) VALUES (1, 1, 1)",
"INSERT INTO prepared_statement_invalidation_test (a, b, c) VALUES (2, 2, 2)",
"INSERT INTO prepared_statement_invalidation_test (a, b, c) VALUES (3, 3, 3)",
"INSERT INTO prepared_statement_invalidation_test (a, b, c) VALUES (4, 4, 4)")) {
"CREATE TABLE prepared_statement_test (a int PRIMARY KEY, b int, c int)",
"INSERT INTO prepared_statement_test (a, b, c) VALUES (1, 1, 1)",
"INSERT INTO prepared_statement_test (a, b, c) VALUES (2, 2, 2)",
"INSERT INTO prepared_statement_test (a, b, c) VALUES (3, 3, 3)",
"INSERT INTO prepared_statement_test (a, b, c) VALUES (4, 4, 4)")) {
sessionRule
.session()
.execute(
Expand All @@ -86,18 +86,61 @@ public void setupSchema() {
}
}

@Test
public void should_have_empty_result_definitions_for_insert_query_without_bound_variable() {
try (CqlSession session = SessionUtils.newSession(ccmRule, sessionRule.keyspace())) {
PreparedStatement prepared =
session.prepare("INSERT INTO prepared_statement_test (a, b, c) VALUES (1, 1, 1)");
assertThat(prepared.getVariableDefinitions()).isEmpty();
assertThat(prepared.getPartitionKeyIndices()).isEmpty();
assertThat(prepared.getResultSetDefinitions()).isEmpty();
}
}

@Test
public void should_have_non_empty_result_definitions_for_insert_query_with_bound_variable() {
try (CqlSession session = SessionUtils.newSession(ccmRule, sessionRule.keyspace())) {
PreparedStatement prepared =
session.prepare("INSERT INTO prepared_statement_test (a, b, c) VALUES (?, ?, ?)");
assertThat(prepared.getVariableDefinitions()).hasSize(3);
assertThat(prepared.getPartitionKeyIndices()).hasSize(1);
assertThat(prepared.getResultSetDefinitions()).isEmpty();
}
}

@Test
public void should_have_empty_variable_definitions_for_select_query_without_bound_variable() {
try (CqlSession session = SessionUtils.newSession(ccmRule, sessionRule.keyspace())) {
PreparedStatement prepared =
session.prepare("SELECT a,b,c FROM prepared_statement_test WHERE a = 1");
assertThat(prepared.getVariableDefinitions()).isEmpty();
assertThat(prepared.getPartitionKeyIndices()).isEmpty();
assertThat(prepared.getResultSetDefinitions()).hasSize(3);
}
}

@Test
public void should_have_non_empty_variable_definitions_for_select_query_with_bound_variable() {
try (CqlSession session = SessionUtils.newSession(ccmRule, sessionRule.keyspace())) {
PreparedStatement prepared =
session.prepare("SELECT a,b,c FROM prepared_statement_test WHERE a = ?");
assertThat(prepared.getVariableDefinitions()).hasSize(1);
assertThat(prepared.getPartitionKeyIndices()).hasSize(1);
assertThat(prepared.getResultSetDefinitions()).hasSize(3);
}
}

@Test
@CassandraRequirement(min = "4.0")
public void should_update_metadata_when_schema_changed_across_executions() {
// Given
CqlSession session = sessionRule.session();
PreparedStatement ps =
session.prepare("SELECT * FROM prepared_statement_invalidation_test WHERE a = ?");
PreparedStatement ps = session.prepare("SELECT * FROM prepared_statement_test WHERE a = ?");
ByteBuffer idBefore = ps.getResultMetadataId();

// When
session.execute(
SimpleStatement.builder("ALTER TABLE prepared_statement_invalidation_test ADD d int")
SimpleStatement.builder("ALTER TABLE prepared_statement_test ADD d int")
.withExecutionProfile(sessionRule.slowProfile())
.build());
BoundStatement bs = ps.bind(1);
Expand All @@ -121,7 +164,7 @@ public void should_update_metadata_when_schema_changed_across_executions() {
public void should_update_metadata_when_schema_changed_across_pages() {
// Given
CqlSession session = sessionRule.session();
PreparedStatement ps = session.prepare("SELECT * FROM prepared_statement_invalidation_test");
PreparedStatement ps = session.prepare("SELECT * FROM prepared_statement_test");
ByteBuffer idBefore = ps.getResultMetadataId();
assertThat(ps.getResultSetDefinitions()).hasSize(3);

Expand All @@ -141,7 +184,7 @@ public void should_update_metadata_when_schema_changed_across_pages() {

// When
session.execute(
SimpleStatement.builder("ALTER TABLE prepared_statement_invalidation_test ADD d int")
SimpleStatement.builder("ALTER TABLE prepared_statement_test ADD d int")
.withExecutionProfile(sessionRule.slowProfile())
.build());

Expand All @@ -168,10 +211,8 @@ public void should_update_metadata_when_schema_changed_across_sessions() {
CqlSession session1 = sessionRule.session();
CqlSession session2 = SessionUtils.newSession(ccmRule, sessionRule.keyspace());

PreparedStatement ps1 =
session1.prepare("SELECT * FROM prepared_statement_invalidation_test WHERE a = ?");
PreparedStatement ps2 =
session2.prepare("SELECT * FROM prepared_statement_invalidation_test WHERE a = ?");
PreparedStatement ps1 = session1.prepare("SELECT * FROM prepared_statement_test WHERE a = ?");
PreparedStatement ps2 = session2.prepare("SELECT * FROM prepared_statement_test WHERE a = ?");

ByteBuffer id1a = ps1.getResultMetadataId();
ByteBuffer id2a = ps2.getResultMetadataId();
Expand All @@ -185,7 +226,7 @@ public void should_update_metadata_when_schema_changed_across_sessions() {
assertThat(rows2.getColumnDefinitions().contains("d")).isFalse();

// When
session1.execute("ALTER TABLE prepared_statement_invalidation_test ADD d int");
session1.execute("ALTER TABLE prepared_statement_test ADD d int");

rows1 = session1.execute(ps1.bind(1));
rows2 = session2.execute(ps2.bind(1));
Expand Down Expand Up @@ -215,10 +256,10 @@ public void should_update_metadata_when_schema_changed_across_sessions() {
public void should_fail_to_reprepare_if_query_becomes_invalid() {
// Given
CqlSession session = sessionRule.session();
session.execute("ALTER TABLE prepared_statement_invalidation_test ADD d int");
session.execute("ALTER TABLE prepared_statement_test ADD d int");
PreparedStatement ps =
session.prepare("SELECT a, b, c, d FROM prepared_statement_invalidation_test WHERE a = ?");
session.execute("ALTER TABLE prepared_statement_invalidation_test DROP d");
session.prepare("SELECT a, b, c, d FROM prepared_statement_test WHERE a = ?");
session.execute("ALTER TABLE prepared_statement_test DROP d");

thrown.expect(InvalidQueryException.class);
thrown.expectMessage("Undefined column name d");
Expand Down Expand Up @@ -250,7 +291,7 @@ private void should_not_store_metadata_for_conditional_updates(CqlSession sessio
// Given
PreparedStatement ps =
session.prepare(
"INSERT INTO prepared_statement_invalidation_test (a, b, c) VALUES (?, ?, ?) IF NOT EXISTS");
"INSERT INTO prepared_statement_test (a, b, c) VALUES (?, ?, ?) IF NOT EXISTS");

// Never store metadata in the prepared statement for conditional updates, since the result set
// can change
Expand Down Expand Up @@ -287,7 +328,7 @@ private void should_not_store_metadata_for_conditional_updates(CqlSession sessio
assertThat(Bytes.toHexString(ps.getResultMetadataId())).isEqualTo(Bytes.toHexString(idBefore));

// When
session.execute("ALTER TABLE prepared_statement_invalidation_test ADD d int");
session.execute("ALTER TABLE prepared_statement_test ADD d int");
rs = session.execute(ps.bind(5, 5, 5));

// Then
Expand Down

0 comments on commit 017053e

Please sign in to comment.